lib: save a reference to intrinsic constructs #12981
Conversation
nodejs-github-bot
added
the
util
refack
added
the
lib / src
|
Not sure what I think of this just yet, but my immediate reaction is that I'd rather we add things as needed rather than try to add a bunch of stuff up front that we're not currently using and might never use. |
|
Oh, also, this definitely needs tests. |
Ack. Just need to figure out how to test it... |
|
I agree we should only add them as needed. |
|
I mentioned snapshotting all intrinsics as a reductio-ad-absurdum argument against saving references to intrinsics, as it seemed to be something impractical to do… |
| @@ -278,3 +278,14 @@ promisify.custom = kCustomPromisifiedSymbol; | |||
|
|
|||
| exports.promisify = promisify; | |||
| exports.customPromisifyArgs = kCustomPromisifyArgsSymbol; | |||
|
|
|||
| exports.intrinsic = { | |||
| FunctionApply: Function.prototype.apply, | |||
There was a problem hiding this comment.
I just realized: it's impossible to actually save a pristine and usable Function.prototype.apply method in JS, since one can just change Function.prototype.apply.call and Function.prototype.apply.apply… That's probably why V8 Extras supply an uncurryThis() function.
There was a problem hiding this comment.
Reflect.apply is the correct answer to this.
| ObjectSetPrototypeOf: Object.setPrototypeOf, | ||
| Object: Object, | ||
| Array: Array, | ||
| ObjectPrototype: Object.assign({}, Object.prototype), |
There was a problem hiding this comment.
I'd rather save the prototype methods individually, and set ObjectPrototype to the initial Object.prototype.
| @@ -278,3 +278,14 @@ promisify.custom = kCustomPromisifiedSymbol; | |||
|
|
|||
| exports.promisify = promisify; | |||
| exports.customPromisifyArgs = kCustomPromisifyArgsSymbol; | |||
|
|
|||
| exports.intrinsic = { | |||
| FunctionApply: Function.prototype.apply, | |||
There was a problem hiding this comment.
Sadly I just realized that util.promisify did not follow the module.exports = {} pattern and should be updated. For new exports from this module, can you use the module.exports = {} above rather than the exports.whatever = ... model?
There was a problem hiding this comment.
I'll be opening a pr to fix up the promisify export, btw
refack
force-pushed
the
save-call-apply
branch
2 times, most recently
from
806c286
to
3963e0f
Compare
|
It seems like a majority thinks that these things should only be added when needed. @refack I guess this should be closed therefore? |
|
This PR should probably be closed; a better solution (if we decide we actually want this) is to expose them using the names from https://tc39.github.io/ecma262/#sec-well-known-intrinsic-objects |
|
+1 to closing this. @refack we can revisit if necessary |
Take a snapshot of some intrinsic constructs, so we can use them when we need to protect against userland monkey patching.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
lib