-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Dates #27
Support Dates #27
Conversation
Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄 |
7a37b13
to
5a06980
Compare
CLA signed! |
5a06980
to
203dc6b
Compare
We detect dates simply by doing an instanceof check on raw value being serialized. We're also now looking at the typeof the *rawValue* not the toJSON()ed value when determining whether a value is a function or a RegExp. This should allow functions and regexs to be serialized properly even if a Function.prototype.toJSON or a RegExp.prototype.toJSON has been added for some reason. Finally, for consistency and because util.isRegExp has been deprecated, we also update our logic for detecting RegExps to use the same strategy as for Dates.
203dc6b
to
3a13d54
Compare
@ericf Are you still involved with this library? If so, can you take a look at this? I'd love to see it merged! |
potential maintainers: @okuryu @juandopazo |
sure. I'll take a look it. |
@redonkulus please add me into git and npm permissions. |
@okuryu done for both |
Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@xumx If I understand correctly, the second serialize call is failing because the RegExp global in your sandbox is different than the RegExp global outside it, right? I.e., it's a cross-realm issue. I see a few options here:
|
Identity discontinuity is a real pain. I don't think providing expansion slots of any kind will help here because we also serialize functions, and if those functions are parsed and evaluated in a different realm, the objects that they produce (imagine that the function returns a regexp) will always exhibit different identity. The solution must be in user-land by evaluating the code in the correct sandbox, or work around the identity discontinuity problems. Side Note: Unfortunately we don't have realms in the language yet (coming soon). |
Continuing this in #31... |
This is a continuation of #16.
To detect Dates, it uses a variant of the most recent strategy proposed by @ericf at #16 (comment). Using that strategy also suggested some tweaks to make the type detection mechanism more robust if Function.prototype.toJSON or RegExp.prototype.toJSON has been set (see commit message).