fix: report semantic strict-mode hazards in ES module output - #21434
Conversation
Sloppy modules emitted as strict ESM can break at runtime with no build-time signal: accessing arguments.callee / arguments.caller throws a TypeError, and assigning to an undeclared variable throws a ReferenceError (a TypeError for read-only globals) instead of creating a global. Report these as warnings, or errors under experiments.futureDefaults, skipping already-strict sources and targets a plugin rewrites. Claude-Session: https://claude.ai/code/session_01TjQJMStkM69Gomi4G5XV5W
|
This PR is packaged and the instant preview is available (1bec5b6). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@1bec5b6
yarn add -D webpack@https://pkg.pr.new/webpack@1bec5b6
pnpm add -D webpack@https://pkg.pr.new/webpack@1bec5b6 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21434 +/- ##
==========================================
+ Coverage 93.17% 93.18% +0.01%
==========================================
Files 610 610
Lines 70785 70836 +51
Branches 20135 20156 +21
==========================================
+ Hits 65951 66010 +59
+ Misses 4834 4826 -8
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:
|
| } | ||
|
|
||
| // The rest keeps its behavior in strict mode — no diagnostics. | ||
| class PrivateAccess { |
There was a problem hiding this comment.
Intentional: PrivateAccess only needs to be parsed, not used — arguments.#p exercises the non-Identifier (private field) property guard in _checkStrictModeArgumentsMember, asserting no diagnostic is emitted. This fixture never executes (the compilation intentionally errors under futureDefaults), so exporting it would add nothing.
Generated by Claude Code
The undeclared-variable warning relied on the parser's definition tracking, which is incomplete inside callback bodies that dependency plugins walk inline without registering params or vars (require.ensure, AMD require/define, import().then) — e.g. a for(var i) loop inside a require.ensure callback was reported as undeclared. Keep only the scope-independent semantic checks: arguments.callee/caller and assignments to the read-only globals undefined/NaN/Infinity. Claude-Session: https://claude.ai/code/session_01TjQJMStkM69Gomi4G5XV5W
|
| Name | Type |
|---|---|
| webpack | Minor |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Merging this PR will improve performance by ×2
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | benchmark "react", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
314 KB | 156.9 KB | ×2 |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/esm-strict-mode-semantic-warnings (9015db5) with main (7f319a9)
|
|
||
| // Already-strict source keeps its behavior in ESM output — no diagnostics. | ||
| function strictCallee() { | ||
| return arguments.callee; |
There was a problem hiding this comment.
Intentional (same as the previous round): this fixture asserts already-strict source gets no diagnostic (scope.isStrict branch); the function is never called. Replacing arguments.callee deletes the tested case.
Generated by Claude Code
|
|
||
| // Already-strict source keeps its behavior in ESM output — no diagnostics. | ||
| function strictCallee() { | ||
| return arguments.callee; |
There was a problem hiding this comment.
Intentional (same as the previous round): asserts already-strict source is skipped by the diagnostics; never called at runtime.
Generated by Claude Code
| } | ||
|
|
||
| function shadowsUndefined() { | ||
| var undefined = 1; |
There was a problem hiding this comment.
Intentional: undefined = 2; is the tested statement — a shadowed undefined binding must suppress the read-only-global diagnostic (the isVariableDefined branch). Collapsing to a single initialization would delete the assignment being asserted.
Generated by Claude Code
| } | ||
|
|
||
| function shadowsUndefined() { | ||
| var undefined = 1; |
There was a problem hiding this comment.
Intentional: undefined = 2; is the tested statement — a shadowed undefined binding must suppress the read-only-global diagnostic (the isVariableDefined branch), and the runnable test asserts the function returns 2. Collapsing the initialization would delete the assignment being asserted.
Generated by Claude Code
Move the detection into StrictModeWarningsPlugin tapping
expressionMemberChain/callMemberChain for("arguments") — the keyed
dispatch already happens, so other expressions pay nothing and the
per-member-expression branches in the walker are gone. Shadowed
bindings never dispatch free-name hooks, replacing the manual check.
Also drop the module-output flag for programs that are already strict
(ESM sources, "use strict"), so the remaining inline checks cost zero
there, and report computed literal access (arguments["callee"]) too.
Claude-Session: https://claude.ai/code/session_01TjQJMStkM69Gomi4G5XV5W
| } | ||
|
|
||
| function shadowsArguments() { | ||
| var arguments = { callee: null }; |
There was a problem hiding this comment.
Intentional (re-flagged from the previous round): the arguments binding is the negative case — a shadowed arguments must suppress the arguments.callee diagnostic (a defined binding never dispatches the free-variable member-chain hook). Renaming it would delete the case being tested; the fixture is parse-only and never runs.
Generated by Claude Code
Since the syntactic checks have no hook points, a separate plugin split the feature across files — register the arguments.callee/caller member-chain taps in the parser itself instead (like its own evaluate taps), keeping the keyed-hook dispatch cost model and one home for all checks. Claude-Session: https://claude.ai/code/session_01TjQJMStkM69Gomi4G5XV5W
…rict-mode diagnostics
Allows configuring the severity of strict-mode violation diagnostics in
ES module output via module.parser.javascript.strictModeViolations
("error" | "warn" | false), globally or per rule. Defaults to "warn",
or "error" under experiments.futureDefaults.
Summary
#21387 reports the strict-mode-only syntax a loose module carries into strict ES module output, but semantic hazards still ship silently:
arguments.callee/arguments.callerthrow a TypeError at runtime, and assigning to the read-only globalsundefined/NaN/Infinitythrows a TypeError instead of silently doing nothing. This reports them through the same mechanism, skipping already-strict sources and shadowed bindings, and adds astrictModeViolationsparser option ("error" | "warn" | false, default"warn","error"underexperiments.futureDefaults) so the diagnostics can be tuned or disabled globally or per rule, mirroringexportsPresence. An undeclared-variable assignment warning was prototyped but dropped: dependency plugins (require.ensure, AMD,import().then) walk callback bodies inline without registering their vars/params, so declaration tracking is structurally incomplete there — reporting it safely first needs those walk paths fixed (possible follow-up). Refs #17121; covers the semantic scope left open by #19628.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/configCases/parsing/strict-mode-module-output-semantics(warnings, bundle still runs),strict-mode-module-output-semantics-future-defaults(errors underexperiments.futureDefaults), andstrict-mode-module-output-severity/-error(per-rulestrictModeViolations: falsesuppression and explicit"error"severity).Does this PR introduce a breaking change?
No — the hazards are reported as warnings by default, only become errors under
experiments.futureDefaults, and can be disabled viastrictModeViolations: false.If relevant, what needs to be documented once your changes are merged or what have you already documented?
Document the new
module.parser.javascript.strictModeViolationsoption and extend the strict-mode diagnostics note foroutput.modulebuilds with the new semantic warnings.Use of AI
Yes — the change and tests were implemented with Claude Code under my direction; I reviewed the diff and verified the test runs (TestCases/ConfigTestCases/StatsTestCases suites,
tsc, lint).Generated by Claude Code