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

ESM hooks used for entry module #41275

Closed
pauldraper opened this issue Dec 22, 2021 · 6 comments · Fixed by #41304
Closed

ESM hooks used for entry module #41275

pauldraper opened this issue Dec 22, 2021 · 6 comments · Fixed by #41304
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders

Comments

@pauldraper
Copy link

pauldraper commented Dec 22, 2021

Version

v17.3.0

Platform

Linux paul 5.4.0-91-generic #102-Ubuntu SMP Fri Nov 5 16:31:28 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

hooks.js

exports.resolve = function (specifier, context, defaultResolve) {
  console.error(specifier, context, defaultResolve);
  return defaultResolve(specifier, context, defaultResolve);
}

main

console.log("Hello world");

Run

node --experimental-loader=./hooks.js ./main

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

Anytime --experimental-loader or --experimental-specifier-resolution is specified.

What is the expected behavior?

I expected the loader to not run for the entry module, since it is CommonJS (the default, no package.json "type":"module").

Indeed, this understanding is supported by the documentation.

When hooks are used they only apply to ES module loading and not to any CommonJS modules loaded.

https://nodejs.org/api/esm.html

What do you see instead?

The loader try to load the main file and breaks.

(node:6928) 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)
file:///home/paul/dev/rivethealth/rules_javascript/main {
  conditions: [ 'node', 'import', 'node-addons' ],
  importAssertions: [Object: null prototype] {},
  parentURL: undefined
} [Function: defaultResolve]
node:internal/errors:464
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension "" for /home/paul/dev/rivethealth/rules_javascript/main
    at new NodeError (node:internal/errors:371:5)
    at Object.file: (node:internal/modules/esm/get_format:72:15)
    at defaultGetFormat (node:internal/modules/esm/get_format:85:38)
    at defaultLoad (node:internal/modules/esm/load:22:14)
    at ESMLoader.load (node:internal/modules/esm/loader:359:26)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:280:58)
    at new ModuleJob (node:internal/modules/esm/module_job:66:26)
    at ESMLoader.#createModuleJob (node:internal/modules/esm/loader:297:17)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:261:34)
    at async Promise.all (index 0) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

Node.js v17.3.0

Additional information

No response

@pauldraper
Copy link
Author

pauldraper commented Dec 22, 2021

I've tried everything, but I can't convince Node.js to load a (CommonJS) entrypoint without an extension, when I'm using either of the two mentioned flags.

Why do I have a entrypoint without an extension? This comes up lots of times.

  • Google Protobuf plugins regard the executable name as significant.
  • Rollup entrypoint is bin/rollup, which has no extension.

@pauldraper
Copy link
Author

Actually, I've found that I can change the loader to explicitly specify CJS.

if (!context.parentURL && path.extname(specifier) == "") {
    return { format: 'commonjs', url: specifier }
}

I tried this on v16 and it still didn't work, but it does work on v17.

@Mesteery Mesteery added esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders labels Dec 22, 2021
@aduh95
Copy link
Contributor

aduh95 commented Dec 22, 2021

I've tried everything, but I can't convince Node.js to load a (CommonJS) entrypoint without an extension, when I'm using either of the two mentioned flags.

This can be achieve using this loader for example:

let entrypoint = true;
export async function load(url, context, defaultLoad) {
  if (entrypoint) {
    context.format = "commonjs";
    entrypoint = false;
  }
  return defaultLoad(url, context, defaultLoad);
}
node --experimental-loader 'data:text/javascript,let%20e=true;export%20function%20load(t,o,r)%7Bif(e)%7Bo.format=%22commonjs%22;e=false%7Dreturn%20r(t,o,r)%7D' ./main

Indeed, this understanding is supported by the documentation.

When hooks are used they only apply to ES module loading and not to any CommonJS modules loaded.

I think the docs are wrong (or outdated), when --experimental-loader is used, the entry point is always passed through the loader hooks – and anything that's imported (imported, not required) after the entry point. @nodejs/loaders should the docs be updated?

@arcanis
Copy link
Contributor

arcanis commented Dec 22, 2021

I think it's a duplicate of #33226, where it's been pinpointed as a bug (which I think is correct, since whatever calls Node doesn't necessarily know whether the entry script is esm or not, they just want it to run fine in both cases).

@pauldraper
Copy link
Author

pauldraper commented Dec 23, 2021

What I don't understand is why using --experimental-loader as a passthrough gives a different result than not specifying the loader.

Node.js can work with ESM and CommonJS just fine, but as soon as you specify a custom option -- even if the custom option is effectively the default -- it's not actually the same.

It's hard to know what even is a bug in Node.js module loading; AFAIK the entrypoint behavior isn't documented.

aduh95 added a commit to aduh95/node that referenced this issue Dec 23, 2021
aduh95 added a commit that referenced this issue Jan 5, 2022
Fixes: #41275

PR-URL: #41304
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jan 14, 2022
Fixes: #41275

PR-URL: #41304
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 31, 2022
Fixes: #41275

PR-URL: #41304
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this issue Jan 31, 2022
Fixes: nodejs#41275

PR-URL: nodejs#41304
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this issue Feb 1, 2022
Fixes: #41275

PR-URL: #41304
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@d3x0r
Copy link

d3x0r commented Sep 3, 2023

But this doesn't actually pass the load through the loader hooks. It only passes the file that node loaded through the hooks, which means the file has to exist on the local disk. If I have a hook that allows loading content over https, then I can't load the main entry point from https.... yes, I see that when the file exists locally, and I load it, then it can use an https://localhost sort of link and resolve the file content... but, again, only when the main entry file first exists on the disk.

M:\javascript\vfs\native>node --experimental-loader ./import.mjs https://localhost:8084/test.mjs 
(node:27928) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

node:internal/process/esm_loader:46
      internalBinding('errors').triggerUncaughtException(
                                ^
Error [ERR_MODULE_NOT_FOUND]: Cannot find module 'M:\javascript\vfs\native\https:\localhost:8084\test.mjs' imported from M:\javascript\vfs\native\
    at new NodeError (node:internal/errors:405:5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders
Projects
None yet
5 participants