ES6 Module Import Regression from Versions 11.3.0 to 11.4.0

[kj800x]

Specs:

• Node.js Version:
  • Tested and working:
    • v11.3.0
  • Tested and broken:
    • v11.4.0
    • v11.5.0
• OS: Tested on Linux Mint 19.1 Cinnamon
  • Linux HOSTNAME 4.15.0-43-generic #46-Ubuntu SMP Thu Dec 6 14:45:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
• Scope (install, code, runtime, meta, other?): ES6 Module Import Regression from v11.3.0 to v11.4.0 using --experimental-modules
• Module (and version) (if relevant): request-promise-native, but stacktrace also points to psl and core node as suspect.

Problem Statement:

Importing request-promise-native using ES6 module syntax on v11.4.0 or v11.5.0 causes a stacktrace which (I think) points to an error in core node:

kevin@rayquaza:~/tmp$ node --experimental-modules index.mjs 
(node:13310) ExperimentalWarning: The ESM module loader is experimental.
TypeError: Cannot read property 'onReady' of undefined
    at Module.load (internal/modules/cjs/loader.js:631:22)
    at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
    at Function.Module._load (internal/modules/cjs/loader.js:552:3)
    at Module.require (internal/modules/cjs/loader.js:659:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/home/kevin/tmp/node_modules/psl/index.js:14:19)
    at Module._compile (internal/modules/cjs/loader.js:723:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:734:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:560:12)

Steps To Reproduce:

• Be on node version v11.4.0 or later:
  • nvm install v11.4.0
• Clone (or otherwise duplicate) the project from this gist: https://gist.github.com/kj800x/f08f44305429bbf8fdd65f9991cb2a71
  • git clone https://gist.github.com/kj800x/f08f44305429bbf8fdd65f9991cb2a71 && cd f08f44305429bbf8fdd65f9991cb2a71
  • It consists of an index file that imports request-promise-native and a package.json and a package-lock.json
• Install dependencies:
  • npm i
• Run the code
  • npm start
    • ACTUAL: Observe the stacktrace and error
• Switch to node version v11.3.0
  • nvm install v11.3.0
• Run the code
  • npm start
    • EXPECTED: Observe that you are able to successfully import the module without errors.

Related

The last three comments on this request-promise-native issue are related: request/request-promise-native#1 (comment)

[guybedford]

Thanks for posting, yes it seems this is a bug in the dynamic modules refactoring.

So the odd thing is that the original PR at https://github.com/nodejs/ecmascript-modules/pull/9/files doesn't have any onReady implementation, while the version that was upstreamed at https://github.com/nodejs/node/pull/24560/files does have this.

//cc @devsnek and @MylesBorins note the differences here:

• Original - https://github.com/nodejs/ecmascript-modules/pull/9/files#diff-76195ce57689942222a27f0dbda6d3b7R629, PR - https://github.com/nodejs/node/pull/24560/files#diff-76195ce57689942222a27f0dbda6d3b7R632
• Original - https://github.com/nodejs/ecmascript-modules/pull/9/files#diff-dc52871f4892835e4b6b339ec0f4b181R36, PR - https://github.com/nodejs/node/pull/24560/files#diff-dc52871f4892835e4b6b339ec0f4b181R43

[devsnek]

@guybedford it was modified slightly since the implementation that landed in ecmascript-modules has bugs. unfortunately all our good tests were removed when they did the "minimal kernel" thing.

[guybedford]

@devsnek ahh so this is definitely the right merge then?

[guybedford]

Let's move this to the main bug tracker - nodejs/node#25482.