Skip to content

esm: preserve syntax error locations in dynamic imports - #66402

Closed
GiHoon1123 wants to merge 1 commit into
nodejs:mainfrom
GiHoon1123:fix/esm-dynamic-import-syntax-error
Closed

GiHoon1123 wants to merge 1 commit into
nodejs:mainfrom
GiHoon1123:fix/esm-dynamic-import-syntax-error

Conversation

@GiHoon1123

Copy link
Copy Markdown

Dynamic imports of modules with syntax errors currently reject with a stack that points into the ESM loader instead of the module source. The compile path already records the source excerpt, but this error is raised before a ModuleJob can decorate the stack.

Decorate translator errors at the loader boundary and cover a caught dynamic import so the filename, line, and source location are retained.

Fixes: #49441

Tests:

  • python3 tools/test.py --mode=release test/es-module/test-esm-syntax-error.mjs test/es-module/test-esm-loader-with-syntax-error.mjs test/es-module/test-require-module-errors.js test/es-module/test-esm-error-cache.js

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Sep 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

Signed-off-by: GiHoon1123 <rlaejrqo465@naver.com>
@GiHoon1123
GiHoon1123 force-pushed the fix/esm-dynamic-import-syntax-error branch from ff63ef4 to 68f2343 Compare September 30, 2026 05:39
@MikeMcC399

Copy link
Copy Markdown
Contributor

The recommendations in:

advise you should tackle only one issue at a time and that you should not open any new PRs until your first PR has been approved.

@GiHoon1123

Copy link
Copy Markdown
Author

Sorry about that - didn't follow the one-issue-at-a-time guideline. Closing this for now and focusing on #64627 first, will reopen once that's merged.

@GiHoon1123 GiHoon1123 closed this Sep 30, 2026
@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.36%. Comparing base (f71d644) to head (68f2343).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66402   +/-   ##
=======================================
  Coverage   90.36%   90.36%           
=======================================
  Files         792      792           
  Lines      275564   275571    +7     
  Branches    52832    52827    -5     
=======================================
+ Hits       249016   249028   +12     
+ Misses      16965    16962    -3     
+ Partials     9583     9581    -2     
Files with missing lines Coverage Δ
lib/internal/modules/esm/loader.js 99.80% <100.00%> (+<0.01%) ⬆️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamic import does not show location of SyntaxError

3 participants