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

--require doesn't work with import #35103

Closed
daveisfera opened this issue Sep 8, 2020 · 20 comments
Closed

--require doesn't work with import #35103

daveisfera opened this issue Sep 8, 2020 · 20 comments
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@daveisfera
Copy link

daveisfera commented Sep 8, 2020

  • Version: 12.18.3
  • Platform: macOS 10.15.6
  • Subsystem:

What steps will reproduce the bug?

Run node with --require and --experimental-specifier-resolution=node

An example can be found here when running npm test

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

There's a way to preload a file using import rather than require

What do you see instead?

internal/modules/cjs/loader.js:1154
      throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/dlj/projects/test_import_preload/preload.js
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1154:13)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Module.require (internal/modules/cjs/loader.js:1026:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1278:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:439:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:71:3)
    at internal/main/run_main_module.js:7:1 {
  code: 'ERR_REQUIRE_ESM'
}
npm ERR! Test failed.  See above for more details.

Additional information

It would be nice if there was a --import or if --require worked with imports

@watilde watilde added the feature request Issues that request new features to be added to Node.js. label Sep 11, 2020
@Trott
Copy link
Member

Trott commented Sep 12, 2020

@nodejs/modules

@devsnek
Copy link
Member

devsnek commented Sep 12, 2020

this is definitely a duplicate but I'm not able to go diving for the original atm.

@daveisfera
Copy link
Author

Is it a duplicate in the sense that there's already a resolution/workaround for this? Or in that this is an existing bug that needs to be addressed?

@jkrems
Copy link
Contributor

jkrems commented Sep 28, 2020

If you need a workaround, you can generally use a small wrapper CJS script that only contains import("./actual.mjs"). The big downside is that it won't run synchronously, if that's something you need.

I don't think anybody is working on support for --import=x.mjs right now. So unless somebody who wants that feature steps up, it's unlikely to happen.

@daveisfera
Copy link
Author

Ya, we're using --require to load settings for our application before the actual app starts and we're using deasync to make sure it's synchronous to accomplish that right now and it works with esm but we'd like to use the built-in support from node, if possible

@jkrems
Copy link
Contributor

jkrems commented Sep 28, 2020

I think adding an --import flag wouldn't be too contentious (I hope). Let me know if you'd like to look into it and want some hints for where to start!

@devsnek
Copy link
Member

devsnek commented Sep 29, 2020

cc @bmeck ^

@targos
Copy link
Member

targos commented Sep 29, 2020

It's probably a very bad idea, but using node --loader something.mjs script.js actually works as a workaround.

@daveisfera
Copy link
Author

@jkrems I've never worked with the code for node, but I'd be willing to give it a try, so we're would I get started?

@targos I gave that a whirl and this was the error I ran into:

(node:20876) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
internal/modules/esm/module_job.js:112
          const namedImports = StringPrototypeMatch(importStatement, /{.*}/)[0];
                                                                            ^

TypeError: Cannot read property '0' of null
    at ModuleJob._instantiate (internal/modules/esm/module_job.js:112:77)
    at async ModuleJob.run (internal/modules/esm/module_job.js:137:5)
    at async Loader.import (internal/modules/esm/loader.js:165:24)
    at async internal/process/esm_loader.js:57:9
    at async Object.loadESM (internal/process/esm_loader.js:67:5)

@devsnek
Copy link
Member

devsnek commented Sep 29, 2020

There was an effort to add --import to node by bmeck, but iirc there were a lot of issues relating to timing.

@bmeck
Copy link
Member

bmeck commented Sep 29, 2020

Yep, I had trouble since everything in the bootstrap assumes it is a synchronous startup, adding asynchronous ticks made things get weird. I'd be wary of using --loader for any sort of global mutation outside of the getGlobalPreloadCode hook. There isn't a guarantee that the Loader will continue to use the same global space as user code.

@jkrems
Copy link
Contributor

jkrems commented Sep 29, 2020

IIRC --loader already got us to a place where we sometimes have ticks during startup. So maybe --import would be possible now, at least for user code where the user can verify whatever timing effect it has?

I'd be wary of using --loader for any sort of global mutation outside of the getGlobalPreloadCode hook.

Yeah, loading arbitrary code in --loader will do weird stuff™. Among other things: It generates a separate module map.

I've never worked with the code for node, but I'd be willing to give it a try, so we're would I get started?

The general starting point would be this guide: https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md. It starts with setting up a local checkout and covers how to test and commit changes.

The --require flag is internally called "preloadModules". For adding a new flag in general, there's a C++ class to register flags.

One possible place to handle --import itself would be around where we also run the --loader-provided global preload code:

async function initializeLoader() {
const { getOptionValue } = require('internal/options');
const userLoader = getOptionValue('--experimental-loader');
if (!userLoader)
return;
let cwd;
try {
cwd = process.cwd() + '/';
} catch {
cwd = 'file:///';
}
// If --experimental-loader is specified, create a loader with user hooks.
// Otherwise create the default loader.
const { emitExperimentalWarning } = require('internal/util');
emitExperimentalWarning('--experimental-loader');
return (async () => {
const hooks =
await ESMLoader.import(userLoader, pathToFileURL(cwd).href);
ESMLoader = new Loader();
ESMLoader.hook(hooks);
ESMLoader.runGlobalPreloadCode();
return exports.ESMLoader = ESMLoader;
})();
}

It may require changing the conditions under which we call initializeLoader.

One important trick to make the development go more quickly: ./configure --node-builtin-modules-path=$PWD. Use that instead of the default ./configure and the node binary you build will load the JS code from your nodejs checkout directly. This can save a lot of time on rebuilding. For this change, you can likely stop rebuilding the binary after the flag is registered.

@devsnek
Copy link
Member

devsnek commented Sep 29, 2020

@jkrems --loader flags are processed after --require flags. But what do you expect --import x --require y --import z to do?

@jkrems
Copy link
Contributor

jkrems commented Sep 29, 2020

But what do you expect --import x --require y --import z to do?

I don't think I'd have strong opinions. To me the order that makes sense is:

  • --require always runs first. It may assume that no ticks have passed etc. and that's hard (or undesirable) for the others.
  • --loader always runs before --import. I assume that --import is supposed to run in the same module map as the rest of the code. So it has to be after --loader.
  • --import always runs last but completes (including TLA) before the first ESM module runs (or before the entrypoint runs?).

The biggest question for me is "should --import also run before a CJS entrypoint". Which I could see either way.

@daveisfera
Copy link
Author

I'm guessing no, but is there another approach to loading data into a global settings object before the rest of the imports happen that is compatible with the ESM support in node 14?

@boneskull
Copy link
Contributor

boneskull commented Oct 8, 2020

(oops, wrong issue)

@daveisfera
Copy link
Author

@jasnell This is still a desired feature, so what's the right way to handle that? Create another issue for it?

@jasnell
Copy link
Member

jasnell commented Jan 6, 2021

Yeah, a separate issue or PR that implements something is ideal

danielleadams pushed a commit that referenced this issue Jan 12, 2021
Fixes: #35103

PR-URL: #36806
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #35103

PR-URL: #36806
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@wmertens
Copy link

what would happen if -r would just mean await import instead of require in a ESM context? I don't think the semantics of the timing matter much for a REPL?

@wmertens
Copy link

wmertens commented Apr 7, 2022

I found a workaround for me, using ioctl to stuff the string to be executed in the TTY before running node:

perl -e 'ioctl(STDIN, 0x5412, $_) for split "", "await import('"'"'$ARGV[0]'"'"')\n";' ./myModule.js; node --loader @node-loader/babel

I'm using Perl because that has ioctl readily availabile.

It would be great if there was a --init flag that you could pass a string that would be executed at the start of the repl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants