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

Experimental modules warning does not show when using dynamic import #30601

Closed
guybedford opened this issue Nov 22, 2019 · 3 comments
Closed
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.

Comments

@guybedford
Copy link
Contributor

As reported by @coreyfarrell here -

Interesting note, the following code executed by CJS does not produce any warning or error in node.js 13.2.0:

import('./nyc.config.mjs')
	.then(console.log)
	.catch(e => console.error(e.message));

When using dynamic import the experimental modules implementation is effectively used "lazily". Even when loaded at this point the warning should still be displayed.

Currently the warning is implemented at https://github.com/nodejs/node/blob/master/lib/internal/process/esm_loader.js#L45.

When calling the first dynamic import through https://github.com/nodejs/node/blob/master/lib/internal/process/esm_loader.js#L26 we should also be able to ensure the warning is provided.

@guybedford guybedford added the esm Issues and PRs related to the ECMAScript Modules implementation. label Nov 22, 2019
@coreyfarrell
Copy link
Member

From looking at this code I suspect we cannot prevent the warning if import('./nyc.config.js') loads a CJS file? This makes me lean towards just not supporting nyc.config.js when type: 'module' is specified.

@guybedford guybedford added good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. labels Nov 25, 2019
@juanarbol
Copy link
Member

How can I recreate this?

I've change esm_loader.js, hard coded warning:

exports.importModuleDynamicallyCallback = async function(wrap, specifier) {
  assert(calledInitialize === true || !userLoader);
  process.emitWarning(
    'The ESM module loader is experimental.',
    'ExperimentalWarning', undefined);
  const { callbackMap } = internalBinding('module_wrap');
  if (callbackMap.has(wrap)) {
    const { importModuleDynamically } = callbackMap.get(wrap);
    if (importModuleDynamically !== undefined) {
      return importModuleDynamically(
        specifier, getModuleFromWrap(wrap) || wrap);
    }
  }
  throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
};
$ ./node mod/file.js
node:6329) ExperimentalWarning: The ESM module loader is experimental.
(node:6329) ExperimentalWarning: The ESM module loader is experimental.
[Module] { default: [Function: default] }

The warning is already emitted on another place, so it appears two times.

@coreyfarrell
Copy link
Member

@juamedgod create two files:

// test.cjs
import('./test.mjs')
  .then(console.log)
  .catch(e => console.error(e.message));
/// test.mjs
export default 'test.mjs';

Then run ./node ./test.cjs. Output on current master branch:

[Module] { default: 'test.mjs' }

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. good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants