Skip to content

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

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

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

Conversation

@GiHoon1123

@GiHoon1123 GiHoon1123 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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

Validation on macOS after rebasing onto main at 768315c:

  • The regression test fails on the main build and passes with this patch.
  • python3 tools/test.py --mode=release -j6 -t60 -p tap es-module: 248 passed, 1 Windows-only test skipped.
  • ESLint for both changed files passed.
  • Repeated imports retain a single source excerpt per error; runtime errors retain their message.

@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.

@GiHoon1123
GiHoon1123 force-pushed the fix/esm-dynamic-import-syntax-error branch from ff63ef4 to 68f2343 Compare September 30, 2026 05:39
@MikeMcC399

This comment was marked as outdated.

@GiHoon1123

Copy link
Copy Markdown
Contributor 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.68%. Comparing base (768315c) to head (a32d994).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66402      +/-   ##
==========================================
- Coverage   90.69%   90.68%   -0.02%     
==========================================
  Files         797      797              
  Lines      279572   279579       +7     
  Branches    53778    53778              
==========================================
- Hits       253569   253533      -36     
- Misses      16287    16343      +56     
+ Partials     9716     9703      -13     
Files with missing lines Coverage Δ
lib/internal/modules/esm/loader.js 99.90% <100.00%> (+<0.01%) ⬆️

... and 37 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.

Decorate errors thrown by module translators before they escape the
loader. This retains source locations for syntax errors caught by a
dynamic import caller, before a ModuleJob can decorate them.

Signed-off-by: GiHoon1123 <rlaejrqo465@naver.com>
Assisted-by: Codex
@GiHoon1123 GiHoon1123 reopened this Nov 9, 2026
@GiHoon1123
GiHoon1123 force-pushed the fix/esm-dynamic-import-syntax-error branch from 68f2343 to a32d994 Compare November 9, 2026 01:47
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