module: add tests for two cases of eager imports:

[MayaLekova]

module: add tests for two cases of eager imports when using import defer

• when the defer imported module contains top-level await
• when the defer imported module is eagerly imported from another module

Refs: https://github.com/tc39/proposal-defer-import-eval
Signed-off-by: Maya Lekova maya@igalia.com

[nicolo-ribaudo]

Note that this is already tested in test262:

• https://github.com/tc39/test262/tree/main/test/language/import/import-defer/evaluation-top-level-await/import-defer-async-module
• https://github.com/tc39/test262/blob/main/test/language/import/import-defer/evaluation-sync/module-imported-defer-and-eager.js

I am not super familiar with the Node.js implementation of ESM, but test262 is run through V8's d8.

Whether or not it makes sense to have these two tests here probably depends on how much they are testing V8-internal code paths vs if there is any part of it that is implemented on the Node.js side.

---

The tests look correct :)

[nicolo-ribaudo]

Oh, could you update the commit first line to mention that it's about import defer?

[joyeecheung]

This doesn't look very Node.js specific and should live in test262 instead? As long as the V8 side works alright I think it's unlikely that anything in particular in Node.js would make it not work.

[MayaLekova]

This doesn't look very Node.js specific and should live in test262 instead? As long as the V8 side works alright I think it's unlikely that anything in particular in Node.js would make it not work.

I was thinking about it and as those 2 tests are really simple, I think it's worth having them in Node.js as well. They are not anymore engine-specific than the initial 2 smoke tests I added in Node. Also makes it really easy to run and - if needed - debug the code in Node directly.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (e6a8d06) to head (ebce535).
⚠️ Report is 293 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64197      +/-   ##
==========================================
- Coverage   92.01%   90.24%   -1.77%     
==========================================
  Files         379      741     +362     
  Lines      166972   240745   +73773     
  Branches    25554    45354   +19800     
==========================================
+ Hits       153639   217270   +63631     
- Misses      13041    15048    +2007     
- Partials      292     8427    +8135     

see 545 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MayaLekova]

Thanks for looking into this!
@jasnell @joyeecheung @caiolima could you please take another look?

[joyeecheung]

I think two small tests are okay to add if they make debugging easier, though we should be cautious about duplicating tests for cases already covered in the upstream, especially when the V8 version on main tends to be months behind the upstream and the proposal can still go through semantic changes..

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74640/

[nodejs-github-bot]

Landed in a006262