-
Notifications
You must be signed in to change notification settings - Fork 826
Assume alwaysStrict #2777
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
base: main
Are you sure you want to change the base?
Assume alwaysStrict #2777
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
Assumes alwaysStrict is effectively always enabled across the compiler/transform pipeline, removing SourceFileAffectingCompilerOptions and updating parsing/binding behavior and test baselines to match.
Changes:
- Remove
SourceFileAffectingCompilerOptionsand associated plumbing from parse/bind/emit paths. - Treat strict mode as always-on for emitted output and for binding non-declaration files.
- Update/trim baselines and skip tests that explicitly set
alwaysStrict=false.
Reviewed changes
Copilot reviewed 300 out of 373 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/transformers/estransforms/usestrict.go | Always inject "use strict" during transform. |
| internal/ast/parseoptions.go | Remove SourceFileAffectingCompilerOptions from parse options and delete helper. |
| internal/binder/binder.go | Bind in strict mode without consulting source-file affecting options. |
| internal/binder/binder_test.go | Update benchmark to reflect removed parse option field. |
| internal/compiler/fileloader.go | Stop computing/passing source-file affecting options; keep external module indicator options. |
| internal/compiler/program.go | Validate that alwaysStrict=false is unsupported and produce a diagnostic. |
| internal/core/compileroptions.go | Remove cached source-file affecting options; keep alwaysStrict only for parsing/validation. |
| internal/project/autoimport.go | Remove passing source-file affecting options while parsing for auto-import registry. |
| internal/project/session_test.go | Adjust tsconfig used in session test. |
| internal/testutil/tsbaseline/js_emit_baseline.go | Remove passing source-file affecting options when parsing in JS emit baselines. |
| internal/testutil/harnessutil/harnessutil.go | Skip tests that explicitly request alwaysStrict=false. |
| testdata/baselines/reference/** | Remove/update reference baselines affected by always-strict behavior and skipped tests. |
| fileParseResult := parser.ParseSourceFile(ast.SourceFileParseOptions{ | ||
| FileName: file.UnitName, | ||
| Path: tspath.Path(file.UnitName), | ||
| CompilerOptions: options.SourceFileAffecting(), | ||
| }, file.Content, core.ScriptKindJSON) |
Copilot
AI
Feb 13, 2026
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.
SourceFileParseOptions no longer receives any options that vary the AST, but the PR description indicates ExternalModuleIndicatorOptions are now the key remaining AST-affecting options. This call site still has access to the test/compiler options (options in this function) but doesn't pass ExternalModuleIndicatorOptions, which can make module detection (and downstream baselines) incorrect for tests that depend on moduleDetection, module, or JSX-driven module-ness. Consider computing and passing ExternalModuleIndicatorOptions here (and any required metadata) so the parse result matches the configured compiler options.
| opts := ast.SourceFileParseOptions{ | ||
| FileName: fileName, | ||
| Path: path, | ||
| CompilerOptions: core.EmptyCompilerOptions.SourceFileAffecting(), | ||
| JSDocParsingMode: ast.JSDocParsingModeParseAll, | ||
| } |
Copilot
AI
Feb 13, 2026
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.
Auto-import parsing now always uses zero-value ExternalModuleIndicatorOptions. Since those options are now the primary remaining AST-affecting parse inputs, this can cause incorrect module classification (especially with moduleDetection: auto/force and JSX-based module forcing), and it can also make the parse cache key insensitive to relevant compiler options. Thread the project’s relevant compiler options into this host (or otherwise supply computed ExternalModuleIndicatorOptions) so cached parses and symbol discovery align with the program’s module detection behavior.
| } | ||
|
|
||
| if options.AlwaysStrict.IsFalse() { | ||
| createRemovedOptionDiagnostic("alwaysStrict", "false", "strict") |
Copilot
AI
Feb 13, 2026
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.
The diagnostic helper name/shape (createRemovedOptionDiagnostic) suggests the option/value was removed, but the behavior here is “alwaysStrict is assumed true; false is unsupported”. This can confuse users (it also points them to "strict", which is not the same concept as ECMAScript strict mode). Consider emitting a more direct message along the lines of “alwaysStrict is always enabled; remove this setting / it cannot be set to false” and avoid implying that switching to strict is equivalent.
| createRemovedOptionDiagnostic("alwaysStrict", "false", "strict") | |
| createRemovedOptionDiagnostic("alwaysStrict", "false", "") |
This follows from microsoft/TypeScript#63089
alwaysStrictis assumed to be true everywhere.This gets rid of the last
SourceFileAffectingCompilerOptionsfield, so that no longer exists.The only compiler options that matter for the AST are now those used for
ExternalModuleIndicatorOptions.All tests which set
alwaysStrict=falsehave been skipped.