-
Notifications
You must be signed in to change notification settings - Fork 734
Split "use strict" into separate transformer, fix bugs with prologues #2028
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors how "use strict" directives are emitted by moving the logic from the CommonJS module transformer into a dedicated UseStrictTransformer. This fixes numerous baseline differences where "use strict" was previously missing or being duplicated in various compilation scenarios.
Key Changes
- Created a new
UseStrictTransformerthat runs before module transformation - Removed
"use strict"emission logic from the CommonJS module transformer - Fixed prologue splitting to correctly handle cases where all statements are prologue
- Updated printer to emit only non-prologue statements when emitting function bodies
Reviewed Changes
Copilot reviewed 300 out of 1181 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/transformers/estransforms/usestrict.go |
New transformer that adds "use strict" based on module type and compiler options |
internal/transformers/moduletransforms/commonjsmodule.go |
Removed "use strict" logic that now belongs in the dedicated transformer |
internal/compiler/emitter.go |
Added the new UseStrictTransformer to the transformation pipeline |
internal/printer/factory.go |
Fixed SplitStandardPrologue to return all statements as prologue when applicable |
internal/printer/printer.go |
Updated to use emitListRange to skip prologue statements in function bodies |
testdata/baselines/reference/**/*.js |
Baseline updates showing "use strict" now correctly appears once at the start of files |
testdata/baselines/reference/**/*.js.diff |
Diff files showing removal of duplicate "use strict" directives |
|
@sheetalkamat I don't really know how to handle this one; the test seems to be flaky. It seems to me like we are not correctly noting that strict/alwaysStrict are emit affecting (or even bind affecting!) |
… race between projects
This fixes a massive number of diffs relating to prologues, including missing
"use strict"or duplicating prologues/comments in general.Before,
"use strict"was added in the CJS transform. But this meant we were not adding it to non-CJS files that also needed it, e.g. scripts.This PR splits the functionality out into a fine-grained transform which handles just adding
"use strict", drops the code from CJS, then fixes related bugs in transforms/emit that were causing things to break with this fix in place.After this PR, there are no missing
"use strict"in any diffs, except for files which are missing entire files or are in general not being detected as ESM correctly. (only 9 diffs last I checked)