module: remove bogus assertion in CJS entrypoint handling with --import #54592

joyeecheung · 2024-08-27T15:52:26Z

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests.

Fixes: #54577

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests.

nodejs-github-bot · 2024-08-27T15:52:32Z

Review requested:

@nodejs/loaders

codecov · 2024-08-27T17:44:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.33%. Comparing base (cc26951) to head (68be620).
Report is 773 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54592      +/-   ##
==========================================
- Coverage   87.34%   87.33%   -0.01%     
==========================================
  Files         649      649              
  Lines      182544   182600      +56     
  Branches    35030    35031       +1     
==========================================
+ Hits       159445   159481      +36     
- Misses      16372    16392      +20     
  Partials     6727     6727

Files with missing lines	Coverage Δ
lib/internal/modules/esm/translators.js	`91.46% <ø> (-0.59%)`	⬇️

... and 56 files with indirect coverage changes

test/es-module/test-require-module-preload.js

mcollina

lgtm

… --import

…ng with --import

joyeecheung · 2024-08-28T22:04:00Z

Happened to notice that this should've used spawnSyncAndAssert(), so fixed that as well

aduh95 · 2024-08-28T22:45:05Z

test/es-module/test-require-module-preload.js

-}
+function testPreload(preloadFlag) {
+  // Test named exports.
+  {


nit: no need for those blocks since we're not declaring any variable

It’s mostly for folding in the editors that support folding blocks.

Doesn't most editors treat multiline function calls as foldable blocks?

In my experience it doesn't always work correctly, but blocks almost never fail.

nodejs-github-bot · 2024-09-07T23:41:08Z

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

nodejs-github-bot · 2024-09-16T10:37:14Z

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

nodejs-github-bot · 2024-09-16T11:57:15Z

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

nodejs-github-bot · 2024-09-16T12:32:45Z

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

joyeecheung · 2024-09-17T10:26:51Z

CI was orange, though the bot seems to fail to communicate that to GitHub again.

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests. PR-URL: #54592 Fixes: #54577 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

joyeecheung · 2024-09-17T10:35:28Z

Landed in 3c4ef34

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests. PR-URL: nodejs#54592 Fixes: nodejs#54577 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests. PR-URL: #54592 Fixes: #54577 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests. PR-URL: nodejs#54592 Fixes: nodejs#54577 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

module: remove bogus assertion in CJS entrypoint handling with --import

76441a7

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests.

joyeecheung mentioned this pull request Aug 27, 2024

ERR_INTERNAL_ASSERTION thrown when using --experimental-require-module and --import together #54577

Closed

nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Aug 27, 2024

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2024

This comment was marked as outdated.

Sign in to view

jasnell approved these changes Aug 27, 2024

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2024

This comment was marked as outdated.

Sign in to view

aduh95 reviewed Aug 28, 2024

View reviewed changes

test/es-module/test-require-module-preload.js Outdated Show resolved Hide resolved

mcollina approved these changes Aug 28, 2024

View reviewed changes

fixup! module: remove bogus assertion in CJS entrypoint handling with…

bc43b1a

… --import

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2024

This comment was marked as outdated.

Sign in to view

fixup! fixup! module: remove bogus assertion in CJS entrypoint handli…

68be620

…ng with --import

aduh95 reviewed Aug 28, 2024

View reviewed changes

aduh95 approved these changes Aug 28, 2024

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024

This comment was marked as outdated.

Sign in to view

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2024

This comment was marked as outdated.

Sign in to view

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2024

This comment was marked as outdated.

Sign in to view

RedYetiDev added the loaders Issues and PRs related to ES module loaders label Sep 14, 2024

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 16, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 16, 2024

joyeecheung closed this Sep 17, 2024

RedYetiDev mentioned this pull request Sep 17, 2024

Proposal: Have the ESM loader handle all entry points #50356

Open

joyeecheung mentioned this pull request Sep 26, 2024

Tracking issue: require(esm) #52697

Open

8 tasks

joyeecheung mentioned this pull request Oct 1, 2024

[v22.x] backport unflagging of --experimental-require-module and related fixes/refactorings #55217

Closed

aduh95 mentioned this pull request Oct 9, 2024

V22.10.0 proposal #55343

Merged

marco-ippolito added the backport-requested-v20.x PRs awaiting manual backport to the v20.x-staging branch. label Nov 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: remove bogus assertion in CJS entrypoint handling with --import #54592

module: remove bogus assertion in CJS entrypoint handling with --import #54592

joyeecheung commented Aug 27, 2024 •

edited

Loading

nodejs-github-bot commented Aug 27, 2024

This comment was marked as outdated.

codecov bot commented Aug 27, 2024 •

edited

Loading

This comment was marked as outdated.

mcollina left a comment

This comment was marked as outdated.

joyeecheung commented Aug 28, 2024

aduh95 Aug 28, 2024

joyeecheung Aug 29, 2024

aduh95 Aug 29, 2024

joyeecheung Aug 29, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Sep 7, 2024

nodejs-github-bot commented Sep 16, 2024

nodejs-github-bot commented Sep 16, 2024

nodejs-github-bot commented Sep 16, 2024

joyeecheung commented Sep 17, 2024

joyeecheung commented Sep 17, 2024

module: remove bogus assertion in CJS entrypoint handling with --import #54592

module: remove bogus assertion in CJS entrypoint handling with --import #54592

Conversation

joyeecheung commented Aug 27, 2024 • edited Loading

nodejs-github-bot commented Aug 27, 2024

This comment was marked as outdated.

codecov bot commented Aug 27, 2024 • edited Loading

Codecov Report

This comment was marked as outdated.

mcollina left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

joyeecheung commented Aug 28, 2024

aduh95 Aug 28, 2024

Choose a reason for hiding this comment

joyeecheung Aug 29, 2024

Choose a reason for hiding this comment

aduh95 Aug 29, 2024

Choose a reason for hiding this comment

joyeecheung Aug 29, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Sep 7, 2024

nodejs-github-bot commented Sep 16, 2024

nodejs-github-bot commented Sep 16, 2024

nodejs-github-bot commented Sep 16, 2024

joyeecheung commented Sep 17, 2024

joyeecheung commented Sep 17, 2024

joyeecheung commented Aug 27, 2024 •

edited

Loading

codecov bot commented Aug 27, 2024 •

edited

Loading