Skip to content
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

Modules: Specify or infer module type for new vm.Script #27387

Closed
azz opened this issue Apr 24, 2019 · 14 comments
Closed

Modules: Specify or infer module type for new vm.Script #27387

azz opened this issue Apr 24, 2019 · 14 comments
Labels
question Issues that look for answers. vm Issues and PRs related to the vm subsystem.

Comments

@azz
Copy link
Contributor

azz commented Apr 24, 2019

Is your feature request related to a problem? Please describe.

Consider the following:

const vm = require('vm');

new vm.Script(`import fs from "fs";`, { filename: 'script.mjs' });

When executed with --experimental-modules, as of 12.0.0 this throws:

script.mjs:1
import fs from "fs";
       ^^

SyntaxError: Unexpected identifier

Describe the solution you'd like

  1. The "input-type" to be inferred from the filename .mjs

  2. An additional option added to vm.Script, called type, which overrides file-based inference.

    new vm.Script(`import fs from "fs";`, { type: 'module' });
  3. Resolve hooks should be available, e.g.:

    async function resolve(specifier, parentURL, defaultResolve) {
      return defaultResolve('./script.mjs', 'file:///');
    }
    new vm.Script(`import fs from "fs"`, { loader: { resolve } });

Describe alternatives you've considered

Alternatively, an API to determine the type/format of a given file path using the same rules that determine the type of node --experimental-modules script.js. E.g.:

const type = require.resolveType('/path/to/file.js');

This would work for some cases but the resolve hook is more powerful.

@devsnek
Copy link
Member

devsnek commented Apr 24, 2019

use vm.SourceTextModule for source text module sources: https://nodejs.org/api/vm.html#vm_class_vm_sourcetextmodule

@devsnek devsnek closed this as completed Apr 24, 2019
@devsnek devsnek added question Issues that look for answers. vm Issues and PRs related to the vm subsystem. labels Apr 24, 2019
@azz
Copy link
Contributor Author

azz commented Apr 24, 2019

Thanks. Is there a mechanism to determine if a module is CJS/ESM? Branching logic will need to introduced to either use vm.Script or vm.SourceTextModule based on this.

@devsnek
Copy link
Member

devsnek commented Apr 24, 2019

there is no logic you can use to reliably determine if a string is a module or a script. as such, node doesn't provide anything. however, you can come up with your own logic that fits your needs. for example if you know that your modules will always contain imports or exports you can use acorn or something to parse and check for them.

@azz
Copy link
Contributor Author

azz commented Apr 24, 2019

I want to follow the same log node uses:

  • If .js, read package.json#type, default to CJS.
  • If .mjs, ESM
  • If .cjs, CJS

@azz
Copy link
Contributor Author

azz commented Apr 24, 2019

Also, trying to get vm.SourceTextModule working, is there a default implementation for linker in module.link(linker)? How is one supposed to return a vm.Module for a built-in, for example?

@devsnek
Copy link
Member

devsnek commented Apr 24, 2019

@azz you can also return a module namespace object, the result of import()

@azz
Copy link
Contributor Author

azz commented Apr 25, 2019

Doesn't seem to be the case?

const vm = require('vm');
(async () => {
  const esModule = new vm.SourceTextModule(`import fs from 'fs';`, {
    url: `file:///script.mjs`,
  });
  await esModule.link(spec => import(spec));
  esModule.instantiate();
  await esModule.evaluate();
})().catch(console.error);

Results in:

(node:54937) ExperimentalWarning: The ESM module loader is experimental.
Error [ERR_VM_MODULE_NOT_MODULE]: Provided module is not an instance of Module
    at ModuleWrap.<anonymous> (internal/vm/source_text_module.js:190:15)
    at processTicksAndRejections (internal/process/task_queues.js:88:5)
    at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:58:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:823:13)
    at internal/main/run_main_module.js:17:11
    at async Promise.all (index 0)
(node:54937) ExperimentalWarning: vm.SourceTextModule is an experimental feature. This feature could change at any time

@devsnek
Copy link
Member

devsnek commented Apr 25, 2019

yeah i'm sorry, the namespace thing is only for dynamic imports. i don't know of a good solution to this off the top of my head, but you can do something like this for now:

import * as fs from 'fs'; // or `const fs = require('fs')`, doesn't really matter
new vm.SourceTextModule(
  Object.keys(fs)
    .map((x) => `export const ${x} = import.meta.mod.${x};`)
    .join('\n'), {
  initializeImportMeta(meta) {
    meta.mod = fs;
  },
});

there are still some things you'll need to take care of, like simulating live bindings.

// Provide named exports for all builtin libraries so that the libraries
// may be imported in a nicer way for ESM users. The default export is left
// as the entire namespace (module.exports) and wrapped in a proxy such
// that APMs and other behavior are still left intact.
NativeModule.prototype.proxifyExports = function() {
this.exportKeys = Object.keys(this.exports);
const update = (property, value) => {
if (this.reflect !== undefined &&
ObjectPrototype.hasOwnProperty(this.reflect.exports, property))
this.reflect.exports[property].set(value);
};
const handler = {
__proto__: null,
defineProperty: (target, prop, descriptor) => {
// Use `Object.defineProperty` instead of `Reflect.defineProperty`
// to throw the appropriate error if something goes wrong.
Object.defineProperty(target, prop, descriptor);
if (typeof descriptor.get === 'function' &&
!Reflect.has(handler, 'get')) {
handler.get = (target, prop, receiver) => {
const value = Reflect.get(target, prop, receiver);
if (ObjectPrototype.hasOwnProperty(target, prop))
update(prop, value);
return value;
};
}
update(prop, getOwn(target, prop));
return true;
},
deleteProperty: (target, prop) => {
if (Reflect.deleteProperty(target, prop)) {
update(prop, undefined);
return true;
}
return false;
},
set: (target, prop, value, receiver) => {
const descriptor = Reflect.getOwnPropertyDescriptor(target, prop);
if (Reflect.set(target, prop, value, receiver)) {
if (descriptor && typeof descriptor.set === 'function') {
for (const key of this.exportKeys) {
update(key, getOwn(target, key, receiver));
}
} else {
update(prop, getOwn(target, prop, receiver));
}
return true;
}
return false;
}
};
this.exports = new Proxy(this.exports, handler);
};

translators.set('builtin', async function builtinStrategy(url) {
debug(`Translating BuiltinModule ${url}`);
// Slice 'node:' scheme
const id = url.slice(5);
const module = NativeModule.map.get(id);
if (!module) {
throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
}
module.compileForPublicLoader(true);
return createDynamicModule(
[...module.exportKeys, 'default'], url, (reflect) => {
debug(`Loading BuiltinModule ${url}`);
module.reflect = reflect;
for (const key of module.exportKeys)
reflect.exports[key].set(module.exports[key]);
reflect.exports.default.set(module.exports);
});
});

@azz
Copy link
Contributor Author

azz commented Apr 25, 2019

Looks like vm.ScriptTextModule is a very low-level API, which is great, but not really what we're after. We want a batteries-included API (like vm.Script) that is effectively running node file.mjs except provides:

  • Ability to intercept and potentially rewire imports, falling back to a default.
  • Ability to run the module in a new context. (Doesn't have to be vm.createContext, just need to define the global this.)

Would there be any interest in providing this? Maybe called new vm.Module?

Basically we want to write something like this:

async function importModule(specifier, parentURL, defaultImport) {
  return defaultImport(specifier, parentURL);
}

const context = vm.createContext();

if (inputIsModule(filePath)) {
  let mod = new vm.Module(code, { filename: filePath, importModule });
  await mod.runInContext(context);
} else {
  let script = new vm.Script(code, { filename: filePath })
  script.runInContext(context);
}

@devsnek
Copy link
Member

devsnek commented Apr 25, 2019

@azz vm.Script and vm.SourceTextModule are the same "level", its just that you need to do more things to run a module than to run a script.

It seems like you want us to expose node's internal module loader, more than a specific vm api, is that right? There are definitely changes we can still make to the SourceTextModule api, that's why it's experimental, but exposing node's internal module loader is kind of no-no territory until we finish designing it.

we want to write something like this

who is "we" and what is the specific use case here? knowing that will help me provide more useful information.

@azz
Copy link
Contributor Author

azz commented Apr 25, 2019

Ah sorry, I didn't explicitly mention (there's an issue reference above). The context is that we're trying to determine if Jest can run tests on files that are ES modules without first transpiling them to CJS via Babel. With CJS, Jest intercepts require() to implement module mocking, and provides a context so it can install fake timers, globals, etc.

Some relevant files:
https://github.com/facebook/jest/blob/master/packages/jest-environment-node/src/index.ts
https://github.com/facebook/jest/blob/master/packages/jest-transform/src/ScriptTransformer.ts

@devsnek
Copy link
Member

devsnek commented Apr 25, 2019

aha, so what you're really looking for is "run node's esm module loader in a new context for jest and also let jest intercept some things"

since we haven't actually settled on how our esm module loader works, you should probably wait a bit to start doing this.

Either we will expose some sort of "loader" api, or you will need to duplicate our behaviour with the vm api.

@azz
Copy link
Contributor Author

azz commented Apr 25, 2019

Yes exactly.

since we haven't actually settled on how our esm module loader works, you should probably wait a bit to start doing this.

Ok no worries, the release of v12 prompted me to see how far I could get with what's already shipped 😄 Thanks for your help 👍

@SimenB
Copy link
Member

SimenB commented Jan 17, 2020

Should we open up an issue in https://github.com/nodejs/modules/issues about this (running a module in a vm.Context) or is it still a bit early for that? I see it's mentioned in the readme over there, but I found no open issues about it.

EDIT: The merged #29864 looks exciting though, the examples seems to cover what we need in Jest 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issues that look for answers. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants