module: add tests for two cases of eager imports:#64197
Conversation
- 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>
nodejs-github-bot
added
esm
There was a problem hiding this comment.
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 :)
| export const foo = 42; | ||
|
|
||
| await setTimeout(1); | ||
| console.log('executed'); |
There was a problem hiding this comment.
Should this also push to eval_list rather than logging?
| // Check that the module has already been evaluated. | ||
| assert.strictEqual(globalThis.eval_list.length, 1); | ||
| assert.strictEqual(ns.foo, 42); | ||
| assert.partialDeepStrictEqual(['defer-tla-1'], globalThis.eval_list); |
There was a problem hiding this comment.
Also assert the length after the ns.foo access.
|
Oh, could you update the commit first line to mention that it's about import defer? |
|
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. |
4 participants
Refs: https://github.com/tc39/proposal-defer-import-eval
Signed-off-by: Maya Lekova maya@igalia.com