standard-things / esm

Tomorrow's ECMAScript modules today!
Other
5.26k stars 146 forks source link

Not working with Node.js 12.11.0? #840

Closed cedx closed 4 years ago

cedx commented 4 years ago

My use case: I have TypeScript projects that are coded as ES modules (for example: cedx/coveralls.js or cedx/which.js). They are intended to run on Node.js v12.x only and use the field "type": "module" in the package.json file (i.e. the used file extension is .js, not .mjs).

Some of their dependencies are also ES modules (in the example: the coveralls.js project depends on the which.js one).

For the tests, I'm using ts-node in conjunction with esm to avoid compiling the projects while I develop them. So, in test environment, the projects are compiled on the fly as CommonJS modules (ts-node does not support ES modules).

This configuration worked fine on Node v12.10.x and below, but since v12.11.0 the tests fail with this error (cf. the CI):

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/runner/work/coveralls.js/coveralls.js/node_modules/@cedx/which/lib/index.js
mikecbrant commented 4 years ago

+1

I filed similar issue with commitlint library, but was guessing the issue might ultimately be here in ESM.

https://github.com/conventional-changelog/commitlint/issues/806

joeldenning commented 4 years ago

+1 I'm seeing this too. Downgrading node fixes this.

newtack commented 4 years ago

+1, have the same issue and downgrading to 12.10 fixed it.

mikecbrant commented 4 years ago

Any update to this? Given the large number of packages that are dependent on this library, this has potential to be a significant problem in the Node ecosystem, as more user update to latest Node version.

jdalton commented 4 years ago

Anyone able to provide more insight into the issue?

mikecbrant commented 4 years ago

@jdalton My particular problem is expalined in more detail in the linked issue. I however don't know enough about how commitlint uses esm or the changes between node 12.10 & 12.11 to provide a better starting point (a brief perusal of node changelog there didn't seem to point to anything around module loader that might be problematic).

All that happened for me was, as soon as I took the 12.11 update to node, my commit linting (which is triggered by husky hooking into git) started to fail.

cedx commented 4 years ago

Minimal code for reproducing the error:

// File: package.json
{
  "type": "module",
  "dependencies": {
    "esm": "^3.2.25"
  }
}

// File: esmodule.js
export const fooBar = 'Hello World!';

// File: main.cjs
const fooBarModule = require('./esmodule.js');
console.dir(fooBarModule.fooBar);

// Command line
node -r esm main.cjs

On Node.js pre-12.11, it prints "Hello World!". On Node.js 12.11+, it triggers this error:

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: esmodule.js

I don't know the internals of Node.js, but it seems that esm is ignored (or takes place too late in the pipeline) by the latest iteration of the module loaders.

Removing the field "type": "module" in package.json fix the error: we can use ES import/export alongside esm in CommonJS modules but not in real ES modules.

newtack commented 4 years ago

@jdalton - would be great if you could look at this

mikecbrant commented 4 years ago

I just got around to updating my NodeJS version to the latest (13.113.0.1) and I am not seeing this problem now. I didn't ever try Node >= 12.12 or 13.0 to know if the problem was encountered with either of those.

cedx commented 4 years ago

@mikecbrant Have you tried the minimal code sample I've provided? I've also upgraded Node.js to version 13.0.1 (v13.1 is not released yet), and the issue is still here on my side.

newtack commented 4 years ago

13.0.1 is now working again for me.

But it emits a warning I did not have with 12.10:

(node:140) Warning: require() of ES modules is not supported. require() of /app/ts/node_modules/zora/dist/bundle/index.js from /app/ts/packages/follow-up-machine/db/lib/tests/index.spec.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules. Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /app/ts/node_modules/zora/package.json.

zora does have "type": "module" as shown in the warning, but it is called with import, not required. I checked the code again to be sure. There is no "require" so not sure why the warning says require().

cedx commented 4 years ago

Closing this issue as it is duplicated by:

855

868

anatoly314 commented 4 years ago

Now it happen in node v13.12.10