ScriptInfo versioning improvements #53001
Conversation
…ent for open file
typescript-bot
added
Author: Team
For Uncommitted Bug
|
Can you merge and update baselines? |
|
@typescript-bot pack this |
|
Heya @andrewbranch, I've started to run the tarball bundle task on this PR at 256f46c. You can monitor the build here. |
|
Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
| @@ -3081,10 +3077,9 @@ declare namespace ts { | |||
| readonly containingProjects: Project[]; | |||
| private formatSettings; | |||
| private preferences; | |||
| private textStorage; | |||
| constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: ScriptInfoVersion); | |||
| constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: number); | |||
There was a problem hiding this comment.
Given this breaking change, is anyone depending on this that we should worry about? Is there a way to write this code as to not break existing users, i.e. keep the old thing around, deprecate, and pull a version out of it?
There was a problem hiding this comment.
I have looked up but didn't find any usage for this which is why i just modified this without adding overload.
You think overload thats marked deprecated is better even then?
There was a problem hiding this comment.
Yeah, now that I'm looking, I've only been able to find 3 examples of this being used (a subset of https://sourcegraph.com/search?q=context:global+/new+.*%5CbScriptInfo%5C%28/+lang:typescript+-file:harness+-file:editorServices+-file:languageServiceHost&patternType=standard&sm=1&groupBy=repo are the right thing), and those all a decade old and unmaintained. So, probably safe.
There was a problem hiding this comment.
So, code wise, I think this looks correct. Just so I understand it, we're effectively removing the double-versioning and now have one version number, consulting ScriptVersionCache only when present, and more lazily creating it?
But @andrewbranch probably will have a better idea of what the intent was.
The other commits are test changes to baseline file version and text for program in the baseline done to ensure the changes dont unexpectedly change versioning and text in the program.
@andrewbranch discovered this while investigating #52792 though as discussed offline it doesnt fix that issue completely.