Skip to content

test: add a simple test for import defer of a CJS module #64694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MayaLekova wants to merge 1 commit into
base: main
Choose a base branch
from
+40 −0
Open

test: add a simple test for import defer of a CJS module #64694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions test/es-module/test-cjs-defer-static-import-eval.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Flags: --js-defer-import-eval

// Test that uses import.defer for a CJS module. It ensures that:
// 1. the module is imported successfully
// 2. it's evaluated synchronously, regardless of the `defer` modifier.

import '../common/index.mjs';
import * as assert from 'assert';

globalThis.eval_list = [];

// Import the CJS module with the `defer` modifier.
// import defer * as imported from '../fixtures/es-modules/package-type-commonjs/index.js';
import defer * as imported from '../fixtures/es-modules/module-cjs-deferred-eval.js';
Comment on lines +10 to +14

@nicolo-ribaudo nicolo-ribaudo Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to swap these two statements, so that when reading somebody does not mistakenly assume that globalThis.eval_list = [] would be evaluated first (imports are hoisted and executed before code in the file).

This test right now would pass regardless of whether import defer is eager or deferred. If it's eager it passes because:

  • after the eager ../fixtures/es-modules/module-cjs-deferred-eval.js evaluation, this file sets globalThis.eval_list = [];. Then it asserts that its length is 0, which is obviously true because we just set it to an empty array.
  • further down, in assert.partialDeepStrictEqual(['defer-1'], globalThis.eval_list);, we are checking that ['defer-1'] contains all the elements of globalThis.eval_list. If globalThis.eval_list is empty, that's tautologically true.

I have not run this test, but I assume the reason in test/fixtures/es-modules/module-cjs-deferred-eval.js you had to add if (!globalThis.eval_list) { globalThis.eval_list = []; } is because the CJS module is indeed not being deferred, so it's running before that this file defines globalThis.eval_list = [].

Maybe better, either one of this:

  • move the globalThis.eval_list = [] initialization to a separate file that is imported at the beginning of this one, before the other imports, so that we know it runs first
  • use a boolean instead of an array, and then we don't need to pre-initialize it.


// Check that the module hasn't been evaluated yet (which is noted
// by adding a property to `globalThis.eval_list`).
assert.strictEqual(globalThis.eval_list.length, 0);

@joyeecheung joyeecheung Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.strictEqual(globalThis.eval_list.length, 0);
assert.deepStrictEqual(globalThis.eval_list, []);


// Check that the exported properties are accessible and have the
// expected values.
assert.equal(imported.default.foo, 42);

Check failure on line 22 in test/es-module/test-cjs-defer-static-import-eval.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md 

'assert.equal' is restricted from being used. Use `assert.strictEqual()` rather than `assert.equal()`
assert.equal(imported.default.identifier, 'package-type-commonjs');

Check failure on line 23 in test/es-module/test-cjs-defer-static-import-eval.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md 

'assert.equal' is restricted from being used. Use `assert.strictEqual()` rather than `assert.equal()`
Comment on lines +22 to +23

@joyeecheung joyeecheung Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.equal(imported.default.foo, 42);
assert.equal(imported.default.identifier, 'package-type-commonjs');
assert.equal(imported.foo, 42);
assert.deepStrictEqual(['defer-1'], globalThis.eval_list);
assert.equal(imported.identifier, 'package-type-commonjs');

The lexer should recognize these. Also we should test that it only gets evaluated once.


// Check that the module has been evaluated at this point.
assert.partialDeepStrictEqual(['defer-1'], globalThis.eval_list);

@joyeecheung joyeecheung Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.partialDeepStrictEqual(['defer-1'], globalThis.eval_list);
assert.deepStrictEqual(['defer-1'], globalThis.eval_list);

It shouldn't be partial, if there are anything else, it's a bug.


// Clean-up
delete globalThis.eval_list;
11 changes: 11 additions & 0 deletions test/fixtures/es-modules/module-cjs-deferred-eval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const identifier = 'package-type-commonjs';
console.log(identifier);

if (!globalThis.eval_list) {
globalThis.eval_list = [];
}
Comment on lines +4 to +6

@jasnell jasnell Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiniest optional nit...

globalThis.eval_list ??= ['defer-1'];


module.exports.foo = 42;
module.exports.identifier = identifier;

globalThis.eval_list.push('defer-1');
Loading