In MODULARIZE mode avoid modifying the incoming moduleArg. NFC #21775
Conversation
| @@ -21,7 +21,7 @@ | |||
| // before the code. Then that object will be used in the code, and you | |||
| // can continue to use Module afterwards as well. | |||
| #if MODULARIZE | |||
| var Module = moduleArg; | |||
| var Module = Object.assign({}, moduleArg); | |||
There was a problem hiding this comment.
Do you think we could take this further and fully separate the arguments and return value at some point too (i.e. not put all the module arg inputs on the module return value)? It would be a bigger breaking change, but IIRC it doesn't seem like many people rely on that behavior when using modularize?
There was a problem hiding this comment.
I see what you mean, yes. I think it might be possible yes.
This avoids leaking of the partially constructed module and means that only way to get access the module instance is via waiting on the promise. Previously we were attaching all our module properties to the incoming object, but not, as far as I can tell for any good reason. In case anybody was actually depending on this, inject thunks into the moduleArg that abort on access with an actionable error.
| var promise = Module(arg); | ||
| arg._foo();''') | ||
|
|
||
| expected = "Aborted(Access to module property ('_foo') is no longer possible via the incoming module contructor argument; Instead, use the result of the module promise" |
| var promise = Module(arg); | ||
| arg._foo();''') | ||
|
|
||
| expected = "Aborted(Access to module property ('_foo') is no longer possible via the incoming module contructor argument; Instead, use the result of the module promise" |
There was a problem hiding this comment.
I read this comment a few times and I'm not actually sure what the user is expected to do to fix it? This is post-js code so it is inside the emitted JS, how can the module Promise be accessed?
Also, this seems like a breaking change that should be mentioned in the changelog?
There was a problem hiding this comment.
This is --extern-post-js code.. where we run the Module() constructor, but we ignore the return value and instead try to use a method on the argument we passed into the constructor.
There was a problem hiding this comment.
Ah, thanks, I missed the -extern-.
- Add changelog entry - Fix typo in error message - Improve error message
This avoids leaking of the partially constructed module and means that only way to get access the module instance is via waiting on the promise.
Previously we were attaching all our module properties to the incoming object, but not, as far as I can tell, for any good reason.
In case anybody was actually depending on this, inject thunks into the moduleArg that abort on access with an actionable error.