-
Notifications
You must be signed in to change notification settings - Fork 30k
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
lib: use safe methods from primordials #27096
Conversation
This changes the primordials to expose built-in prototypes with their methods already uncurried. The uncurryThis function is therefore moved to the primordials. All uses of uncurryThis on built-ins are changed to import the relevant prototypes from primordials. All uses of Function.call.bind are also changed to use primordials.
Co-Authored-By: targos <targos@protonmail.com>
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.
I borrowed the term primordials from the realms proposal, I believe it makes sense to have your own primordials other than the JS builtins? But anyway that's just academic..
Does this affect performance at all? From what I can tell this does not seem to be on any path that's performance-sensitive, though. |
I don't know which one is faster between Or are you talking about the property access ( |
if (!Reflect.getOwnPropertyDescriptor(dest, key)) { | ||
const desc = Reflect.getOwnPropertyDescriptor(src, key); | ||
if (typeof desc.value === 'function') { | ||
desc.value = uncurryThis(desc.value); |
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.
I was talking about this, as this affect all prototype methods, not just the Function ones.
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.
It is already what was done manually in all the files I changed (either with call.bind or uncurryThis). This is centralizing the uncurrying to this file. If we want to use the methods safely, we have to call them without going through the prototype.
Landed in 112cc7c |
This changes the primordials to expose built-in prototypes with their methods already uncurried. The uncurryThis function is therefore moved to the primordials. All uses of uncurryThis on built-ins are changed to import the relevant prototypes from primordials. All uses of Function.call.bind are also changed to use primordials. PR-URL: #27096 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This changes the primordials to expose built-in prototypes with their
methods already uncurried.
The uncurryThis function is therefore moved to the primordials.
All uses of uncurryThis on built-ins are changed to import the relevant
prototypes from primordials.
All uses of Function.call.bind are also changed to use primordials.