Breakage in Node.js v20.6.0

[mcollina]

The new loader API in v20.6.0 broke esmock

   FAIL  "should return un-mocked file"
    Cannot find package '[object Object]?esmk=1' imported from /Users/matteo/Repositories/esmock/src/esmock.js

    at nextResolveCall (file:///Users/matteo/Repositories/esmock/src/esmockLoader.js:71:7)
    at resolve (file:///Users/matteo/Repositories/esmock/src/esmockLoader.js:78:12)

I did try a quick replacement with the new initialize, but it looked a bit harder than expected.

cc @GeoffreyBooth for visibility.

[koshic]

issue comes from here:

esmockModule -> moduleId, parent ../local/usesImportObjects.js file:///xxxxxx/esmock/tests/tests-node/esmock.node.global.test.js
esmockModule -> moduleFileURL {}
esmockModule -> moduleFileURL to string [object Object]

[koshic]

Another interesting point is nodejs/node#49028

"Unflags import.meta.resolve making the second parentURL argument still gated behind the same --experimental-import-meta-resolve flag so that existing users would retain backwards compatibility, while the default behaviour becomes standards compliant."

So we have 2 different behaviors of import.meta.resolve depending on experimental flag. And import.meta.resolve is always available.

I can successfully run a few tests with '--experimental-import-meta-resolve' and dirty patched resolve:

But there is a new error:

[iambumblehead]

I do have time to investigate a solution today. Feel free to give any suggestions that should be considered.

[iambumblehead]

@koshic thanks for finding the cause and writing the nice summary. I understand.

[koshic]

the native resolve function is broken in node 20.6

With --experimental-import-meta-resolve flag?

[koshic]

mkdir -p xxx && touch xxx/1.mjs && touch xxx/2.mjs && \
echo 'import {resolve} from "path"; console.log(import.meta.resolve("./1.mjs", "file://" + resolve("./xxx/2.mjs")))' > 3.mjs && \
node --experimental-import-meta-resolve 3.mjs

works as expected (== returns file URL for 1.mjs) in 20.5.0 & 20.6.0

[iambumblehead]

@koshic thanks eventually your solution will be used in this PR #236

[iambumblehead]

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

It seems import.meta.resolve will discontinue support for the 'parent' param, but indirectly continues support (for now) when the node process includes the flag --experimental-import-meta-resolve

Failing tests commented-out at the PR were caused by import.meta.resolve's lack of support for "parent". For example, this test does not resolve the eslint module relative to parent.

const fileURL = resolve.constructor.name === 'AsyncFunction'
    ? await resolve(id, parent) : resolve(id, parent)
// node v20.6, fileURL is incorrect, test fails
//  file:///home/bumble/software/esmock/node_modules/eslint/lib/api.js
// node v20.4, fileURL is correct, test passes
//  file:///home/bumble/software/esmock/tests/node_modules/eslint/lib/api.js

@koshic @mcollina what do you think? Maybe import.meta.resolve should be removed from esmock and the scripted resolvewithplus used instead. Most users are using resolvewithplus already anyway, because import.meta.resolve has been undefined without the experimental flag.

[iambumblehead]

Another issue affecting node v20.6: #237

The "load" hook is called with a nextLoad function that returns { source: null } for CJS modules in node v20.6

looping in @tommy-mitchell

[iambumblehead]

https://nodejs.org/api/esm.html#loadurl-context-nextload

The value of source can be omitted for type 'commonjs'. When a source is provided, all require calls from this module will be processed by the ESM loader with registered resolve and load hooks; [...] This behavior for nullish source is temporary — in the future, nullish source will not be supported.

[iambumblehead]

Based on investigation, node v20.6 partially allows the loader to process cjs trees imported at the global-CJS-imports test, however, unknown factors cause the process to fail when certain cjs trees are imported multiple times.

The failing test uses a package called "meow". Possibly, the meow package could be opened and dependency trees imported there disabled and enabled to learn more information about the problem.

[iambumblehead]

I think the problem is meow has a circular dependency

[iambumblehead]

Some progress. There are no circular dependencies in meow.

esmock defined global values at require js sources, updating the source file to look like this,

const { Date } = require('file:///import?esmk=4') // something like this...

// ...rest of the file

Newer versions of node have problems loading the path given to require. Error messages say the file is not found. Because 'file://import' is not a file, esmock was updated locally to use a known path instead, something like 'file:///home/bumble/esmock/esmockDummy.js?import?esmk=4' and this also caused the file not found error. An actual error,

✖ should optionally catch CJS circular imports (30.518101ms)
  Error: Cannot find module 'file:///home/bumble/software/esmock/src/esmockDummy.js?import?esmkTreeId=1&esmkModuleId=import&isfound=false&isesm=false&exportNames=console'
  Require stack:
  - /home/bumble/software/esmock/tests/node_modules/map-obj/index.js
      at Module._resolveFilename (node:internal/modules/cjs/loader:1048:15)
      at require (node:internal/modules/esm/translators:177:30)
      at Object.<anonymous> (/home/bumble/software/esmock/tests/node_modules/map-obj/index.js:1:19)
      at loadCJSModule (node:internal/modules/esm/translators:207:3)
      at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:233:7)
      at ModuleJob.run (node:internal/modules/esm/module_job:217:25)
      at async ModuleLoader.import (node:internal/modules/esm/loader:308:24)
      at async file:///home/bumble/software/esmock/src/esmock.js:12:26
      at async TestContext.<anonymous> (file:///home/bumble/software/esmock/tests/tests-node/esmock.node.global.test.js:84:3)
      at async Test.run (node:internal/test_runner/test:582:9) {
    code: 'MODULE_NOT_FOUND',
    requireStack: [ '/home/bumble/software/esmock/tests/node_modules/map-obj/index.js' ]
  }

When a commonjs source like the one at the top of this comment is returned, the error occurs right away and the resolve hook is never called.

Replacing the require call with an object definition gets around the issue,

const { Date } = global.mocks('file:///import?esmk=4')

// ...rest of the file

But there is yet another issue at the test using the meow package. This is the error

(node:157711) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
✖ should work when modules have CJS imports (354.79829ms)
  SyntaxError [Error]: /home/bumble/software/esmock/tests/node_modules/spdx-license-ids/index.json: Unexpected token 'i', "import {co"... is not valid JSON
      at parse (<anonymous>)
      at ModuleLoader.jsonStrategy (node:internal/modules/esm/translators:416:21)
      at callTranslator (node:internal/modules/esm/loader:265:14)
      at ModuleLoader.<anonymous> (node:internal/modules/esm/loader:269:24)
      at new ModuleJob (node:internal/modules/esm/module_job:65:26)
      at #createModuleJob (node:internal/modules/esm/loader:282:17)
      at ModuleLoader.getJobFromResolveResult (node:internal/modules/esm/loader:240:34)
      at ModuleLoader.getModuleJobSync (node:internal/modules/esm/loader:226:17)
      at require (node:internal/modules/esm/translators:191:36)
      at Object.<anonymous> (/home/bumble/software/esmock/tests/node_modules/spdx-expression-parse/scan.js:4:11)

[iambumblehead]

Tests are passing at the branch that removes import.meta.resolve #237 To get around the error described in the last message, the load hook is scripted to only modify sources with format 'module' or 'commonjs'

One of the typescript tests is commented-out. Tests using the 'meow' package, including the commented-out typescript test, are big and not focused. I believe the problem with the failing test exists in the test sources, but I've run out of energy for it and believe the current PR improves things and resolves issues at node v20.6.

If we revisit the typescript test, we might want to break it into smaller focused tests.

Please review @koshic @tommy-mitchell if you are available

[iambumblehead]

if import.meta.resolve is dropped, lets publish to new minor version 2.4.0

[koshic]

Maybe import.meta.resolve should be removed from esmock

Agree - browser-like version of import.meta.resolve is useless for our purposes.

[iambumblehead]

esmock 2.4.0 is published

[iambumblehead]

The commented-out tests were affected by another issue: node's loader sometimes returns { source: undefined } and other times returns { source: null }

A solution for that issue is at the PR here #238 and will likely be published soon.