Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up`vm.Script`'s `importModuleDynamically` should get a reference to the `context` used to run it #35714
Comments
|
( |
|
We should add the needed properties to scripts. Extending the |
|
That would mean the passed in const assert = require('assert');
const vm = require('vm');
const outerScript = new vm.Script('import("woo").then(console.log)', {
async importModuleDynamically(_, innerScript) {
assert.ok(outerScript === innerScript);
// the below is just to please the API in returning a module, not relevant to the point I'm making
const module = new vm.SyntheticModule(['default'], function () {
this.setExport('default', 'hello!');
});
await module.link(() => {
throw new Error('Linker should not be called');
});
await module.evaluate();
return module;
},
});
outerScript.runInThisContext();If These APIs are experimental, so I guess it doesn't really matter if that contract (if it even is a contract and not a "coincidence") is broken. I would personally prefer an approach where it's added to |
|
oh yeah i guess thats why i didn't add a context property to it. i'll have to think about this a bit more. |
Is your feature request related to a problem? Please describe.
When implementing
importModuleDynamicallyyou don't have access to what context the script is executed with, meaning you can't pass the correct context when constructing aModule. We're also missing thefilenameof theScript, so resolving to the specifier passed is also not straightforward since it can be changed by.runIn*Context()calls from what I passed in to the constructor.Describe the solution you'd like
I think adding
contextandfilenameas accessible properties on theScriptinstance passed to the function should work fine - it would mirror what you get when usingSourceTextModulewhere I have access tocontextandidentifier. It could also be passed as a third argument to the function passed inimportModuleDynamicallyif you don't wanna change theScriptinstance itself.Describe alternatives you've considered
The implementation I've gone with in the absence of such an API is to get the context again and re-use the filename passed in the constructor. This works since I also have control over how the script is executed, but that might not always be the case. Mirroring the capability of
SourceTextModulewould be nice, though - where in the context of the callback inimportModuleDynamicallythere's enough information to know how to resolve the specifier being requested and in what context it should run.