fix: report strict-mode-only syntax in ES module output - #21387
Conversation
When a loose (CommonJS/script) module is emitted as strict-mode ES module output, acorn had already skipped the strict early errors, so constructs that become a runtime SyntaxError (delete of a variable, `with`, octal literals and escapes, duplicate parameters, assigning to `eval`/`arguments`) silently produced a broken bundle. Report them as a warning, or an error under `experiments.futureDefaults`.
🦋 Changeset detectedLatest commit: 135a047 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 (ee439ad). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@ee439ad
yarn add -D webpack@https://pkg.pr.new/webpack@ee439ad
pnpm add -D webpack@https://pkg.pr.new/webpack@ee439ad |
Types CoverageCoverage after merging claude/webpack-17121-verify-4ghheq into main will be
Coverage Report |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21387 +/- ##
==========================================
+ Coverage 92.81% 92.97% +0.16%
==========================================
Files 601 604 +3
Lines 68222 69166 +944
Branches 19195 19474 +279
==========================================
+ Hits 63318 64307 +989
+ Misses 4904 4859 -45
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:
|
| var notOctalA = "\\048"; // escaped backslash, not an octal escape | ||
| var notOctalB = "\0"; // NUL escape, valid | ||
|
|
||
| function dup(a, a) { |
| var notOctalA = "\\048"; // escaped backslash, not an octal escape | ||
| var notOctalB = "\0"; // NUL escape, valid | ||
|
|
||
| function dup(a, a) { |
| var obj = { x: 1 }; | ||
| delete obj.x; // member delete stays valid, must not be reported | ||
|
|
||
| with (obj) { |
| var obj = { x: 1 }; | ||
| delete obj.x; // member delete stays valid, must not be reported | ||
|
|
||
| with (obj) { |
| // Pure CommonJS module: parsed as a loose script, so acorn does not reject | ||
| // these strict-mode-only violations — but the ESM output runs in strict mode. | ||
| var foo = 2; | ||
| delete foo; |
| // Pure CommonJS module: parsed as a loose script, so acorn does not reject | ||
| // these strict-mode-only violations — but the ESM output runs in strict mode. | ||
| var foo = 2; | ||
| delete foo; |
| } | ||
|
|
||
| eval = 1; | ||
| arguments = 2; |
| } | ||
|
|
||
| eval = 1; | ||
| arguments = 2; |
| var octalNumber = 0777; | ||
| var octalStringA = "\047"; // octal escape (\0 followed by a digit) | ||
| var octalStringB = "\47"; // octal escape (\1-\7) | ||
| var notOctalA = "\\048"; // escaped backslash, not an octal escape |
| var octalStringA = "\047"; // octal escape (\0 followed by a digit) | ||
| var octalStringB = "\47"; // octal escape (\1-\7) | ||
| var notOctalA = "\\048"; // escaped backslash, not an octal escape | ||
| var notOctalB = "\0"; // NUL escape, valid |
Merging this PR will improve performance by 30.87%
Performance Changes
Tip Curious why this is faster? Comment Comparing |
Summary
A loose (CommonJS/script) module is parsed in sloppy mode, so acorn skips the strict-mode early errors. When such a module is emitted as strict-mode ES module output (
output.module), constructs likedelete <variable>,with, octal literals/escapes, duplicate parameters, and assigning toeval/argumentssilently produced a bundle that throws aSyntaxErrorat runtime, with no build-time diagnostic. This reports them during the build. Refs #17121 (the "delete xshould be a build error as code is compiled in strict mode" item).What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/configCases/parsing/strict-mode-module-output(warning by default) andtest/configCases/parsing/strict-mode-module-output-future-defaults(error underexperiments.futureDefaults).Does this PR introduce a breaking change?
No. The violations are reported as a warning by default; they only become an error under
experiments.futureDefaults(opt-in today, the default in the next major).If relevant, what needs to be documented once your changes are merged or what have you already documented?
The new strict-mode warnings for
output.modulebuilds, and their escalation to errors underexperiments.futureDefaults.Use of AI
Yes. The parser diagnostics, the two integration test cases, and the changeset were drafted with Claude (Claude Code); I verified the behavior (false-positive checks, full config test suite,
tsc/lint) and reviewed the diff before submitting.Generated by Claude Code