fix(parser): report strict-mode-invalid binding names in ES module output - #21567
Conversation
…tput A loose script keeps binding names that strict mode rejects, so acorn accepts them, but emitting the module as ES module output makes the whole bundle fail to parse — previously with no diagnostic at all. Extend strictModeViolations to bindings named eval/arguments, the strict-mode reserved words, and await (reserved by the module goal), covering variable declarations, function/class declaration names, named function expressions and catch parameters.
🦋 Changeset detectedLatest commit: 2caec48 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR is packaged and the instant preview is available (2d18aa6). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@2d18aa6
yarn add -D webpack@https://pkg.pr.new/webpack@2d18aa6
pnpm add -D webpack@https://pkg.pr.new/webpack@2d18aa6 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21567 +/- ##
==========================================
+ Coverage 93.85% 93.92% +0.06%
==========================================
Files 622 623 +1
Lines 75561 75656 +95
Branches 21989 22030 +41
==========================================
+ Hits 70920 71059 +139
+ Misses 4641 4597 -44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 8.64%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "asset-modules-resource", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
668.1 KB | 1,241.2 KB | -46.18% |
| ❌ | Memory | benchmark "future-defaults", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
234.8 KB | 349.2 KB | -32.76% |
| ⚡ | Memory | benchmark "asset-modules-source", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
1,367.7 KB | 649 KB | ×2.1 |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/strict-mode-binding-names (2caec48) with main (1fdea24)
… words
Parameters and named class expressions are bindings too, so the same names
break there: a loose script accepts `function f(static)` and
`var C = class await {}`, but ES module output rejects both.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c0d8599. Configure here.
Summary
A loose script keeps binding names that strict mode rejects, so acorn accepts them, but emitting the module as ES module output makes the whole bundle fail to parse — previously with no diagnostic at all. This extends
strictModeViolationsto bindings namedeval/arguments, the strict-mode reserved words, andawait(reserved by the module goal, so it gets its own message), covering variable declarations, function/class declaration names, named function expressions and catch parameters. Refs #17121, and closes the gap left by #21387/#21434.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/configCases/parsing/strict-mode-module-output-bindings(12 diagnostics across every binding form), plus an added expectation instrict-mode-module-output-semantics-future-defaults, where shadowingargumentsis itself a strict-mode SyntaxError.Does this PR introduce a breaking change?
No — reported as warnings by default, errors only under
experiments.futureDefaults, and suppressible withstrictModeViolations: false.If relevant, what needs to be documented once your changes are merged or what have you already documented?
Extend the
strictModeViolationsnote with the binding-name diagnostics.Use of AI
Yes. Claude Code was used to enumerate the strict-mode early errors from the spec, build fixtures proving each one breaks the emitted bundle, implement the checks and write the tests; I reviewed the diff and verified the runs before submitting.
Generated by Claude Code
Note
Medium Risk
Touches hot parse paths in JavascriptParser for every binding form when ESM output is enabled; behavior is gated by strictModeViolations and only affects loose modules emitted as strict ESM.
Overview
Extends
strictModeViolationsso loose source that would fail to parse once emitted as strict ES module output gets a warning or error instead of a silent broken bundle.JavascriptParsernow flags binding names that strict mode or module goal reject:eval,arguments, strict reserved words (static,yield,package, etc.), andawait(with a distinct module-goal message). Checks run when_strictInModuleOutputis set—variable and destructuring declarations, function/class declaration names, parameters (including arrows), catch bindings, named function expressions, class expressions, and IIFE paths that bypasswalkFunctionExpression. Parameter checking also covers reserved words andawait, not onlyeval/arguments.Diagnostics still honor
strictModeViolations(warn vs error vs off). A new config-case asserts the expected warnings;strict-mode-module-output-semantics-future-defaultsadds an expectation that shadowingargumentsis reported as an invalid binding.Reviewed by Cursor Bugbot for commit 2caec48. Bugbot is set up for automated code reviews on this repo. Configure here.