(fix) prevent ts plugin crash when config updated

[jasonlyu123]

#2018 sveltejs/kit#9932

This happens when tsconfig or extending tsconfig updates and another ts-plugin returns another language service object. For example, https://marketplace.visualstudio.com/items?itemName=VisualStudioExptTeam.vscodeintellicode. Because the patched symbol is no longer there, patching will run again. And when we call project.addRoot, a Debug assertion error will be thrown as the file is already a root file. TypeScript will then removes the external files because the plugin failed to initialise.

[dummdidumm]

Is the if (!configManager.getConfig().enable || p === 'dispose') { below still correct?

[jasonlyu123]

Hmm. I don't remember why dispose has special treatment here. But it seems wrong. Maybe at some point in the original PR dispose is patched before proxy is created, or something.

[jasonlyu123]

I remember why dispose has special treatment here now. It should run the decorated version even if the typescript-plugin is disabled, but the current check is wrong.

[dummdidumm]

This happens when tsconfig or extending tsconfig updates and another ts-plugin returns another language service object

another language service object which still has our patches applied? Because if not, what would happen how?

[jasonlyu123]

Another language service object which still has our patches applied.

Yes. But not the same object. Debugging show that they use Object.keys() to copy over unpatched method/property to the new object. So the symbol is not copied over.

[dummdidumm]

Do they just copy over everything? In other words, if we didn't use a symbol but a __svelte-language-tools__ property it would also work? I guess we're somewhat stuck between false positives or false negatives, and the question is which is worse.

[jasonlyu123]

if we didn't use a symbol but a __svelte-language-tools__ property it would also work?

Yes. But TypeScript will copy over that property no matter if the plugin has extended our patch (proxy Inception lol).
https://github.com/microsoft/TypeScript/blob/24ac9e75c2f04f9084fcd21df7d3fabc7dee9fe0/src/server/project.ts#L2005

Once we add that property, it'll be there until the language service is destroyed. So it's not much better than checking patched project names. But it's a more direct check. I can change to that if you prefer that.

[dummdidumm]

If it's moved over due to the key thing anyway, then this change is probably for the better 👍

[jasonlyu123]

@Princesseuh FYI, Astro might also have this problem.