-
Notifications
You must be signed in to change notification settings - Fork 718
Cache CompilerOptions.SourceFileAffecting #1156
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 PR introduces caching of the SourceFileAffecting
result on CompilerOptions
(enabled by #1106) and updates call sites and tests accordingly.
- Add
sync.Once
and a cached field toCompilerOptions
and updateSourceFileAffecting
to initialize once. - Remove per-
Program
caching and invoke the cached compiler options directly. - Update tests in
tsconfigparsing_test.go
andcommandlineparser_test.go
to ignore the new unexported fields. - Remove a stale
// TODO
comment infileloader.go
.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
internal/tsoptions/tsconfigparsing_test.go | Imported cmpopts and updated DeepEqual to ignore unexported fields |
internal/tsoptions/commandlineparser_test.go | Same import and DeepEqual update for command-line parser tests |
internal/core/compileroptions.go | Added caching fields and modified SourceFileAffecting to use sync.Once |
internal/compiler/program.go | Removed Program-level caching and updated callers to use new cache |
internal/compiler/fileloader.go | Removed outdated caching TODO comment |
Comments suppressed due to low confidence (3)
internal/core/compileroptions.go:380
- Consider adding tests to verify the caching behavior of
SourceFileAffecting
, ensuring that repeated calls return the same instance and that concurrency safety holds.
return options.sourceFileAffectingCompilerOptions
internal/core/compileroptions.go:364
- [nitpick] Add a doc comment to explain that
SourceFileAffecting
caches its result on theCompilerOptions
instance, and note any implications ifCompilerOptions
is mutated afterward.
func (options *CompilerOptions) SourceFileAffecting() *SourceFileAffectingCompilerOptions {
internal/core/compileroptions.go:367
- By caching the
SourceFileAffectingCompilerOptions
on first call, subsequent calls will not reflect any modifications toCompilerOptions
. Consider invalidating or regenerating the cached value if options can change after initialization, or document that options must remain immutable after first use.
options.sourceFileAffectingCompilerOptionsOnce.Do(func() {
Enabled by #1106, we can now cache the SourceFileAffecting function's results on the CompilerOptions struct itself.