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 75.00000% with 21 lines in your changes missing coverage. Please review.

Project coverage is 90.36%. Comparing base (8c2df73) to head (de697ab).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/esm/loader.js	69.81%	15 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.34%   90.36%   +0.01%     
==========================================
  Files         629      629              
  Lines      184403   184449      +46     
  Branches    36040    36037       -3     
==========================================
+ Hits       166607   166677      +70     
- Misses      10919    10928       +9     
+ Partials     6877     6844      -33     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/module_job.js	99.01% <80.00%> (-0.25%)	⬇️
src/module_wrap.cc	76.49% <84.61%> (+0.73%)	⬆️
lib/internal/modules/esm/loader.js	96.24% <69.81%> (-0.78%)	⬇️

... and 32 files with indirect coverage changes

[nodejs-github-bot]

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