Skip to content

feat: warn when modules are automatically converted to strict mode #19628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: warn when modules are automatically converted to strict mode #19628

wants to merge 8 commits into from

Conversation

@3ru
Copy link
Member

@3ru 3ru commented Jun 25, 2025 •
edited
Loading

This PR introduces strict-mode compatibility diagnostics to help developers safely migrate modules that will be executed in strict mode and/or emitted as ESM.

Rel #17121

What kind of change does this PR introduce?

  • New plugin: StrictCompatibilityPlugin
    • Hooks into the JavaScript parsing pipeline and reports only strict-mode-relevant issues
    • Leverages parser.isVariableDefined for exact scope checks
    • Emits diagnostics via module.addWarning / addError depending on strictCompatibility mode
  • Configuration/schema/types:
    • Adds strictCompatibility to module.parser.javascript schema and d.ts:
      • "warn" | "error" | false
    • Applied in WebpackOptionsApply (no-op unless configured)

Detection (ECMA-262 Annex C)

Strictness-sensitive patterns are classified into:

  • Syntax-incompatible in strict mode (treated as build errors or parse failures)
    • with statements
    • delete on unqualified Identifier (delete x)
    • Duplicate function parameters
    • Using eval/arguments as identifiers
    • Legacy octal numeric literals (e.g., 0644) — handled by Acorn as parse errors in strict mode
    • Legacy octal and non-octal decimal escape sequences (e.g., "\123", "\8", "\9") — handled by Acorn as parse errors in strict mode
  • Semantic/runtime-impactful (surfaced as warnings by default)
    • arguments.callee / arguments.caller
    • Accessing Function#caller / Function#arguments
    • Top-level this (becomes undefined in strict mode)
  • Assignments and updates (errors in strict mode)
    • Assignment to undeclared identifiers (detected via parser scope)
    • Assignment to read-only globals: undefined, Infinity, NaN
    • Assignment/update of eval or arguments
    • Catch parameter named arguments

Note: Some of these are rejected by Acorn in strict mode before plugin hooks (they appear as Module parse failed), which is expected and relied upon.

Did you add tests for your changes?

yes

Does this PR introduce a breaking change?

no

Might be good to write docs?

@3ru 3ru marked this pull request as ready for review June 25, 2025 16:54
@3ru
Copy link
Member Author

3ru commented Jun 25, 2025

I'm not sure if this matches the original intent of the TODO, so I'd appreciate your review and guidance.

@codspeed-hq
Copy link

codspeed-hq bot commented Jun 25, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #19628 will degrade performances by 30.97%

Comparing 3ru:feat/warn-strict-mode-conversion (01051d4) with main (1ffd83f)

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 40 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
⚡ benchmark "devtool-eval", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 113.6 ms 41.2 ms ×2.8
❌ benchmark "devtool-source-map", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 77.7 ms 112.5 ms -30.97%

@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch 2 times, most recently from bca8a2b to 5318930 Compare June 26, 2025 03:52
@3ru
Copy link
Member Author

3ru commented Jul 18, 2025

@alexander-akait
FYI, This is

Inform (warn) the user about modules that are converted to strict mode

as discussed in 17121.

@alexander-akait
Copy link
Member

alexander-akait commented Jul 22, 2025 •
edited
Loading

@3ru This is a slightly different task, we need to check if a module can be converted to strict mode (i.e. and so to ES modules too) and if not - inform developer about it, so the main goal here - is write function which check AST on bad modules (which can be converted) and when check it can be converted to ESM, we can split this task into 2 tasks

@3ru
Copy link
Member Author

3ru commented Aug 10, 2025 •
edited
Loading

@alexander-akait Should we add this warning feature via a plugin, or are we building it directly into the core?
Also, I improved the implementation to check most cases that would error in strict mode(please refer PR description). I'm dividing roles with the parts that cause parse errors on the Acorn side.

ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#transitioning_to_strict_mode

3ru added 3 commits August 10, 2025 16:42
@3ru
feat: warn when modules are automatically converted to strict mode
6e8e4df 
- Add WarnStrictModeModulesPlugin to detect modules converted to strict mode

- Track modules with explicit 'use strict' to avoid false warnings

- Plugin is opt-in to avoid breaking existing builds
@3ru
chore: cleanup
2181b79
@3ru
feat(parser): strict-mode compatibility diagnostics
ea2182a
@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch from 5318930 to ea2182a Compare August 10, 2025 23:49
@3ru 3ru marked this pull request as draft August 10, 2025 23:49
@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch from 698d6f4 to e516bc5 Compare August 11, 2025 01:05
@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch from e516bc5 to bcfb3d8 Compare August 11, 2025 01:11
@alexander-akait
Copy link
Member

alexander-akait commented Aug 11, 2025

@3ru Great job, honestly automatic transformation would be useful, but it take the time, so right now I think we can -

  1. Add experimental.strictCompatibility (maybe better name)
  2. Add parser.javascript.strictCompatibility by default false, it can be true only when experimental.strictCompatibility, like we do for output.module

@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch 6 times, most recently from 75bb77c to 8d749b7 Compare August 16, 2025 08:19
@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch 2 times, most recently from bf82e5f to c17eb5e Compare August 16, 2025 22:46
@3ru
Copy link
Member Author

3ru commented Aug 16, 2025 •
edited
Loading

✅ Confirmed module link errors after 24.6. Working on 24.5. Will remove the
584c818 commit before merge.

@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch from c17eb5e to 584c818 Compare August 16, 2025 23:55
@3ru 3ru marked this pull request as ready for review August 17, 2025 00:30
@3ru 3ru force-pushed the feat/warn-strict-mode-conversion branch from 584c818 to 23fb6a5 Compare August 17, 2025 19:04
3ru added 2 commits August 17, 2025 12:05
@3ru
Merge branch 'main' into feat/warn-strict-mode-conversion
925d266
@3ru
Merge branch 'main' into feat/warn-strict-mode-conversion
01051d4 
Resolved conflicts in:
- declarations/WebpackOptions.d.ts: Kept strictModeChecks property and added extensibility with [k: string]: any
- schemas/WebpackOptions.check.js: Regenerated with yarn fix:special
@3ru
Copy link
Member Author

3ru commented Sep 3, 2025

I'll resolve the conflicts after review, since we keep running into them.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@3ru @alexander-akait