module: handle cached linked async jobs in require(esm)

[joyeecheung]

This handles two cases caused by using Promise.all() with multiple dynamic import() that can make an asynchronously linked module job that has finished/failed linking but has not yet started actual evaluation appear in the load cache when another require request is in turn to handle it. It's similar to how we handle the case when the two are both asynchronous.

• When the cached async job has finished linking but has not started its evaluation because the async run() task would be later in line, start the evaluation from require() with runSync(). The evaluation, being synchronous, is guaranteed to complete by the time the other run() is about to start evaluation, the other task would just get the cached evaluation results instead of re-evaluating it, similar to what would happen if both are asynchronously imported.
• When the cached async job have already encountered a linking error that gets wrapped into a rejection, but is still later in line to throw on it, just unwrap and throw the linking error from require().

Refs: shufo/prettier-plugin-blade#311
Refs: https://github.com/fisker/prettier-plugin-blade-311
Refs: mochajs/mocha#5290
Refs: https://github.com/JoshuaKGoldberg/repros/tree/mocha-missing-module-cyclic
Fixes: #57172

[nodejs-github-bot]

Review requested:

• @nodejs/loaders
• @nodejs/vm

[joyeecheung]

I think it's still possible to have cached async jobs yielding JS-land handling while another require(esm) comes in and pick up the cache for the same module. In those cases, we can't really handle them as we can't turn a linking process that has started asynchronously to switch to a synchronous route. To get rid of these edge cases once and for all, we should do at least part of #55782 and make the linking fully synchronous. But I haven't seen those edge cases so far and the refactoring would be more complex to complete. Until then, this fixes the two cases that we can handle without refactoring and have appeared in the user land.

I don't think I can minimally reproduce the shape of the microtask queue of the two cases, as they depend on how linking and evaluation tasks from a complex graph happen to be interleaved. So I didn't add any tests, but I can confirm locally it fixes https://github.com/fisker/prettier-plugin-blade-311 and https://github.com/JoshuaKGoldberg/repros/tree/mocha-missing-module-cyclic

[codecov]

Codecov Report

Attention: Patch coverage is 69.38776% with 30 lines in your changes missing coverage. Please review.

Project coverage is 90.23%. Comparing base (47a59bd) to head (be776fa).
Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/esm/loader.js	62.68%	24 Missing and 1 partial ⚠️
src/module_wrap.cc	84.61%	2 Missing and 2 partials ⚠️
lib/internal/modules/esm/module_job.js	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57187      +/-   ##
==========================================
- Coverage   90.24%   90.23%   -0.01%     
==========================================
  Files         630      630              
  Lines      184920   184980      +60     
  Branches    36186    36198      +12     
==========================================
+ Hits       166874   166920      +46     
- Misses      11059    11088      +29     
+ Partials     6987     6972      -15     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/module_job.js	98.27% <80.00%> (-0.22%)	⬇️
src/module_wrap.cc	72.45% <84.61%> (+0.22%)	⬆️
lib/internal/modules/esm/loader.js	95.04% <62.68%> (-1.64%)	⬇️

... and 34 files with indirect coverage changes

[nodejs-github-bot]

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

[guybedford]

Would be great to see this backported.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[mcollina]

lgtm

[nodejs-github-bot]

Landed in 0aa3ddd