Simplify parser options passing, share ASTs even more #1158
Conversation
| func (h *compilerHost) GetSourceFile(opts ast.SourceFileParseOptions) *ast.SourceFile { | ||
| text, ok := h.FS().ReadFile(opts.FileName) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| if tspath.FileExtensionIs(fileName, tspath.ExtensionJson) { | ||
| return parser.ParseJSONText(fileName, path, text) | ||
| } | ||
| return parser.ParseSourceFile(fileName, path, text, options, metadata, scanner.JSDocParsingModeParseForTypeErrors) | ||
| return parser.ParseSourceFile(opts, text, core.GetScriptKindFromFileName(opts.FileName)) | ||
| } |
There was a problem hiding this comment.
Note that the ScriptKind and the text are not parameters to GetSourceFile (not options). Implementations of GetSourceFile are in charge of fetching the file contents, and then they also override ScriptKind depending. Here, the core compiler only cares about the path to things, but in the LS the ScriptKind is determined by the language server (since the editor can say "actually, this is JS").
| if scriptKind == core.ScriptKindUnknown { | ||
| panic("ScriptKind must be specified when parsing source files.") | ||
| } |
There was a problem hiding this comment.
Allowing the compiler to determine the ScriptKind itself is a problem because it means we save onto the SourceFiles the "wrong" value, which means that using it for keys in DocumentRegistry and so on fails later because those caches are unaware of what the parser used to do to it.
| type ExternalModuleIndicatorOptions struct { | ||
| jsx bool | ||
| force bool | ||
| } |
There was a problem hiding this comment.
Shifting this code around now means that the entire cache key for the external module indicator is just these two booleans! So much more reuse.
| func GetSourceFileAffectingCompilerOptions(fileName string, options *core.CompilerOptions) core.SourceFileAffectingCompilerOptions { | ||
| // Declaration files are not parsed/bound differently depending on compiler options. | ||
| if tspath.IsDeclarationFileName(fileName) { | ||
| return core.SourceFileAffectingCompilerOptions{} |
There was a problem hiding this comment.
It turns out that it's entirely safe to not use any options (source affecting, external module indicator) at all for declaration files, so we can now share these ASTs everywhere.
jakebailey
changed the title
This PR adds
ast.SourceFileParseOptions, a value type which is the input to file parsing. The parser now takes it instead of many parameters that have to be plumbed around, and then stores a copy on the SourceFile for later use in map keys and similar. This greatly simplifiesGetSourceFileimplementors.Additionally, we're now able to produce better options (really, cache keys) for parsing. Declaration files now set no options at all, so can be fully shared between projects no matter what options are set. Regular source files have fewer options to be able to set the external module indicator (just two booleans). Further follow-up PRs will reduce this set even more.
The optimizations in this PR lead to the peak memory in
testrunnerto drop from 13GB to ~4GB.