From 285830075624d8fbfc591f3b14f5269ff8b987a5 Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Mon, 18 Nov 2024 12:11:48 +0800 Subject: [PATCH 1/5] fix error when script tag is removed in node16/nodenext --- .../src/plugins/typescript/module-loader.ts | 32 +----- .../src/plugins/typescript/service.ts | 71 ++++++++---- .../test/plugins/typescript/service.test.ts | 106 ++++++++++++++++++ 3 files changed, 156 insertions(+), 53 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts index 4cbde00b8..74f822e6c 100644 --- a/packages/language-server/src/plugins/typescript/module-loader.ts +++ b/packages/language-server/src/plugins/typescript/module-loader.ts @@ -7,8 +7,7 @@ import { ensureRealSvelteFilePath, getExtensionFromScriptKind, isSvelteFilePath, - isVirtualSvelteFilePath, - toVirtualSvelteFilePath + isVirtualSvelteFilePath } from './utils'; const CACHE_KEY_SEPARATOR = ':::'; @@ -89,8 +88,6 @@ class ModuleResolutionCache { } class ImpliedNodeFormatResolver { - private alreadyResolved = new FileMap>(); - constructor(private readonly tsSystem: ts.System) {} resolve( @@ -106,39 +103,17 @@ class ImpliedNodeFormatResolver { let mode: ReturnType = undefined; if (sourceFile) { - this.cacheImpliedNodeFormat(sourceFile, compilerOptions); mode = ts.getModeForResolutionAtIndex(sourceFile, importIdxInFile, compilerOptions); } return mode; } - private cacheImpliedNodeFormat(sourceFile: ts.SourceFile, compilerOptions: ts.CompilerOptions) { - if (!sourceFile.impliedNodeFormat && isSvelteFilePath(sourceFile.fileName)) { - // impliedNodeFormat is not set for Svelte files, because the TS function which - // calculates this works with a fixed set of file extensions, - // which .svelte is obv not part of. Make it work by faking a TS file. - if (!this.alreadyResolved.has(sourceFile.fileName)) { - sourceFile.impliedNodeFormat = ts.getImpliedNodeFormatForFile( - toVirtualSvelteFilePath(sourceFile.fileName) as any, - undefined, - this.tsSystem, - compilerOptions - ); - this.alreadyResolved.set(sourceFile.fileName, sourceFile.impliedNodeFormat); - } else { - sourceFile.impliedNodeFormat = this.alreadyResolved.get(sourceFile.fileName); - } - } - } - resolveForTypeReference( entry: string | ts.FileReference, - sourceFile: ts.SourceFile | undefined, - compilerOptions: ts.CompilerOptions + sourceFile: ts.SourceFile | undefined ) { let mode = undefined; if (sourceFile) { - this.cacheImpliedNodeFormat(sourceFile, compilerOptions); mode = ts.getModeForFileReference(entry, sourceFile?.impliedNodeFormat); } return mode; @@ -315,8 +290,7 @@ export function createSvelteModuleLoader( const entry = getTypeReferenceResolutionName(typeDirectiveName); const mode = impliedNodeFormatResolver.resolveForTypeReference( entry, - containingSourceFile, - options + containingSourceFile ); const key = `${entry}|${mode}`; diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index 47321a531..7bf224143 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -26,7 +26,8 @@ import { findTsConfigPath, getNearestWorkspaceUri, hasTsExtensions, - isSvelteFilePath + isSvelteFilePath, + toVirtualSvelteFilePath } from './utils'; import { createProject, ProjectService } from './serviceCache'; import { internalHelpers } from 'svelte2tsx'; @@ -1370,38 +1371,60 @@ function getOrCreateDocumentRegistry( registry = ts.createDocumentRegistry(useCaseSensitiveFileNames, currentDirectory); - // impliedNodeFormat is always undefined when the svelte source file is created - // We might patched it later but the registry doesn't know about it - const releaseDocumentWithKey = registry.releaseDocumentWithKey; - registry.releaseDocumentWithKey = ( + const acquireDocumentWithKey = registry.acquireDocumentWithKey; + registry.acquireDocumentWithKey = ( + fileName: string, path: ts.Path, + compilationSettingsOrHost: ts.CompilerOptions | ts.MinimalResolutionCacheHost, key: ts.DocumentRegistryBucketKey, - scriptKind: ts.ScriptKind, - impliedNodeFormat?: ts.ResolutionMode + scriptSnapshot: ts.IScriptSnapshot, + version: string, + scriptKind?: ts.ScriptKind, + sourceFileOptions?: ts.CreateSourceFileOptions | ts.ScriptTarget ) => { - if (isSvelteFilePath(path)) { - releaseDocumentWithKey(path, key, scriptKind, undefined); - return; - } - - releaseDocumentWithKey(path, key, scriptKind, impliedNodeFormat); - }; + const compilationSettings = getCompilationSettings(compilationSettingsOrHost); + const host: ts.MinimalResolutionCacheHost | undefined = + compilationSettingsOrHost === compilationSettings + ? undefined + : (compilationSettingsOrHost as ts.MinimalResolutionCacheHost); + if ( + host && + isSvelteFilePath(fileName) && + typeof sourceFileOptions === 'object' && + !sourceFileOptions.impliedNodeFormat + ) { + const format = ts.getImpliedNodeFormatForFile( + toVirtualSvelteFilePath(fileName), + host?.getCompilerHost?.()?.getModuleResolutionCache?.()?.getPackageJsonInfoCache(), + host, + compilationSettings + ); - registry.releaseDocument = ( - fileName: string, - compilationSettings: ts.CompilerOptions, - scriptKind: ts.ScriptKind, - impliedNodeFormat?: ts.ResolutionMode - ) => { - if (isSvelteFilePath(fileName)) { - registry?.releaseDocument(fileName, compilationSettings, scriptKind, undefined); - return; + // sourceFileOptions.impliedNodeFormat = format; } - registry?.releaseDocument(fileName, compilationSettings, scriptKind, impliedNodeFormat); + return acquireDocumentWithKey( + fileName, + path, + compilationSettingsOrHost, + key, + scriptSnapshot, + version, + scriptKind, + sourceFileOptions + ); }; documentRegistries.set(key, registry); return registry; + + function getCompilationSettings( + settingsOrHost: ts.CompilerOptions | ts.MinimalResolutionCacheHost + ) { + if (typeof settingsOrHost.getCompilationSettings === 'function') { + return (settingsOrHost as ts.MinimalResolutionCacheHost).getCompilationSettings(); + } + return settingsOrHost as ts.CompilerOptions; + } } diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index 360848fad..19cffe7bd 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -281,6 +281,112 @@ describe('service', () => { }); }); + it('do not throw when script tag is nuked', async () => { + // testing this because the patch rely on ts implementation details + // and we want to be aware of the changes + + const dirPath = getRandomVirtualDirPath(testDir); + const { virtualSystem, lsDocumentContext, rootUris } = setup(); + + virtualSystem.writeFile( + path.join(dirPath, 'tsconfig.json'), + JSON.stringify({ + compilerOptions: { + module: 'NodeNext', + moduleResolution: 'NodeNext' + } + }) + ); + + virtualSystem.writeFile( + path.join(dirPath, 'random.svelte'), + '' + ); + virtualSystem.writeFile( + path.join(dirPath, 'random2.svelte'), + '' + ); + + const ls = await getService( + path.join(dirPath, 'random.svelte'), + rootUris, + lsDocumentContext + ); + + const document = new Document(pathToUrl(path.join(dirPath, 'random.svelte')), ''); + document.openedByClient = true; + ls.updateSnapshot(document); + + const document2 = new Document( + pathToUrl(path.join(dirPath, 'random2.svelte')), + virtualSystem.readFile(path.join(dirPath, 'random2.svelte'))! + ); + document.openedByClient = true; + ls.updateSnapshot(document2); + + const lang = ls.getService(); + lang.getProgram(); + + document2.update(' ', 0, document2.getTextLength()); + ls.updateSnapshot(document2); + assert.doesNotThrow(() => { + lang.getProgram(); + }); + }); + + it.only('do not throw when lang="ts" is nuke while a new files is added', async () => { + // testing this because the patch rely on ts implementation details + // and we want to be aware of the changes + + const dirPath = getRandomVirtualDirPath(testDir); + const { virtualSystem, lsDocumentContext, rootUris } = setup(); + + virtualSystem.writeFile( + path.join(dirPath, 'tsconfig.json'), + JSON.stringify({ + compilerOptions: { + module: 'NodeNext', + moduleResolution: 'NodeNext' + } + }) + ); + + virtualSystem.writeFile( + path.join(dirPath, 'random.svelte'), + '' + ); + virtualSystem.writeFile( + path.join(dirPath, 'random2.svelte'), + '' + ); + + const ls = await getService( + path.join(dirPath, 'random.svelte'), + rootUris, + lsDocumentContext + ); + + const document = new Document(pathToUrl(path.join(dirPath, 'random.svelte')), ''); + document.openedByClient = true; + ls.updateSnapshot(document); + + const document2 = new Document( + pathToUrl(path.join(dirPath, 'random2.svelte')), + virtualSystem.readFile(path.join(dirPath, 'random2.svelte'))! + ); + document.openedByClient = true; + ls.updateSnapshot(document2); + + const lang = ls.getService(); + lang.getProgram(); + + document2.update(' ', 0, document2.getTextLength()); + ls.updateSnapshot(document2); + ls.openVirtualDocument(new Document(pathToUrl(path.join(dirPath, 'random3.svelte')), '')); + + lang.getProgram(); + }); + function createReloadTester( docContext: LanguageServiceDocumentContext, testAfterReload: (reloadingConfigs: string[]) => Promise From ab6df4d524a48ade48070bf18ff3e76872b73a8e Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Sun, 22 Dec 2024 11:06:19 +0800 Subject: [PATCH 2/5] need to have syntax error --- .../language-server/test/plugins/typescript/service.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index 19cffe7bd..18d2b15c0 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -327,11 +327,9 @@ describe('service', () => { const lang = ls.getService(); lang.getProgram(); - document2.update(' ', 0, document2.getTextLength()); + document2.update(' { - lang.getProgram(); - }); + ls.getService(); }); it.only('do not throw when lang="ts" is nuke while a new files is added', async () => { From a33c782c1085f3f4dcb94ab2425dd4bf9521ee59 Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Mon, 23 Dec 2024 12:41:38 +0800 Subject: [PATCH 3/5] two more cases tracked it in git so we can reenable it later --- .../test/plugins/typescript/service.test.ts | 103 +++++++++++++----- 1 file changed, 77 insertions(+), 26 deletions(-) diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index 18d2b15c0..656de8b5d 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -332,57 +332,108 @@ describe('service', () => { ls.getService(); }); - it.only('do not throw when lang="ts" is nuke while a new files is added', async () => { + it.skip('do not throw when ScriptKind changes before new service is created', async () => { // testing this because the patch rely on ts implementation details // and we want to be aware of the changes const dirPath = getRandomVirtualDirPath(testDir); const { virtualSystem, lsDocumentContext, rootUris } = setup(); + const package1Path = path.join(dirPath, 'package1'); + const package2Path = path.join(dirPath, 'package2'); virtualSystem.writeFile( - path.join(dirPath, 'tsconfig.json'), + path.join(package1Path, 'tsconfig.json'), JSON.stringify({ compilerOptions: { - module: 'NodeNext', - moduleResolution: 'NodeNext' + allowJs: true } }) ); virtualSystem.writeFile( - path.join(dirPath, 'random.svelte'), - '' + path.join(package2Path, 'tsconfig.json'), + JSON.stringify({ + compilerOptions: { + allowJs: true + } + }) ); + + const usagePath = path.join(package1Path, 'usage.svelte'); virtualSystem.writeFile( - path.join(dirPath, 'random2.svelte'), - '' + usagePath, + '' ); - const ls = await getService( - path.join(dirPath, 'random.svelte'), - rootUris, - lsDocumentContext + const libraryPath = path.join(package2Path, 'Library.svelte'); + virtualSystem.writeFile(libraryPath, ''); + + const lsLibrary = await getService(libraryPath, rootUris, lsDocumentContext); + lsLibrary.getService(); + + const documentLibrary = new Document( + pathToUrl(libraryPath), + virtualSystem.readFile(libraryPath)! ); + documentLibrary.openedByClient = true; + documentLibrary.update(' ', 0, documentLibrary.getTextLength()); + lsLibrary.updateSnapshot(documentLibrary); + - const document = new Document(pathToUrl(path.join(dirPath, 'random.svelte')), ''); - document.openedByClient = true; - ls.updateSnapshot(document); + const lsUsage = await getService(usagePath, rootUris, lsDocumentContext); + lsUsage.getService(); + }); - const document2 = new Document( - pathToUrl(path.join(dirPath, 'random2.svelte')), - virtualSystem.readFile(path.join(dirPath, 'random2.svelte'))! + it.skip('do not throw when ScriptKind changes referenced service is updated', async () => { + // testing this because the patch rely on ts implementation details + // and we want to be aware of the changes + + const dirPath = getRandomVirtualDirPath(testDir); + const { virtualSystem, lsDocumentContext, rootUris } = setup(); + const package1Path = path.join(dirPath, 'package1'); + const package2Path = path.join(dirPath, 'package2'); + + virtualSystem.writeFile( + path.join(package1Path, 'tsconfig.json'), + JSON.stringify({ + compilerOptions: { + allowJs: true + } + }) ); - document.openedByClient = true; - ls.updateSnapshot(document2); - const lang = ls.getService(); - lang.getProgram(); + virtualSystem.writeFile( + path.join(package2Path, 'tsconfig.json'), + JSON.stringify({ + compilerOptions: { + allowJs: true + } + }) + ); - document2.update(' ', 0, document2.getTextLength()); - ls.updateSnapshot(document2); - ls.openVirtualDocument(new Document(pathToUrl(path.join(dirPath, 'random3.svelte')), '')); + const usagePath = path.join(package1Path, 'usage.svelte'); + virtualSystem.writeFile( + usagePath, + '' + ); - lang.getProgram(); + const libraryPath = path.join(package2Path, 'Library.svelte'); + virtualSystem.writeFile(libraryPath, ''); + + const lsLibrary = await getService(libraryPath, rootUris, lsDocumentContext); + lsLibrary.getService().getProgram(); + const lsUsage = await getService(usagePath, rootUris, lsDocumentContext); + lsUsage.getService(); + + const documentLibrary = new Document( + pathToUrl(libraryPath), + virtualSystem.readFile(libraryPath)! + ); + documentLibrary.openedByClient = true; + documentLibrary.update(' ', 0, documentLibrary.getTextLength()); + lsLibrary.updateSnapshot(documentLibrary); + + lsLibrary.getService(); }); function createReloadTester( From bbcdd939d9a21e0a9998362e2462814c745b9ce8 Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Mon, 23 Dec 2024 12:45:15 +0800 Subject: [PATCH 4/5] cleanup --- .../src/plugins/typescript/service.ts | 2 +- .../test/plugins/typescript/service.test.ts | 104 ------------------ 2 files changed, 1 insertion(+), 105 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index e4d9a6280..fe2ff3a91 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -1406,7 +1406,7 @@ function getOrCreateDocumentRegistry( compilationSettings ); - // sourceFileOptions.impliedNodeFormat = format; + sourceFileOptions.impliedNodeFormat = format; } return acquireDocumentWithKey( diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index 656de8b5d..d2832bc33 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -332,110 +332,6 @@ describe('service', () => { ls.getService(); }); - it.skip('do not throw when ScriptKind changes before new service is created', async () => { - // testing this because the patch rely on ts implementation details - // and we want to be aware of the changes - - const dirPath = getRandomVirtualDirPath(testDir); - const { virtualSystem, lsDocumentContext, rootUris } = setup(); - const package1Path = path.join(dirPath, 'package1'); - const package2Path = path.join(dirPath, 'package2'); - - virtualSystem.writeFile( - path.join(package1Path, 'tsconfig.json'), - JSON.stringify({ - compilerOptions: { - allowJs: true - } - }) - ); - - virtualSystem.writeFile( - path.join(package2Path, 'tsconfig.json'), - JSON.stringify({ - compilerOptions: { - allowJs: true - } - }) - ); - - const usagePath = path.join(package1Path, 'usage.svelte'); - virtualSystem.writeFile( - usagePath, - '' - ); - - const libraryPath = path.join(package2Path, 'Library.svelte'); - virtualSystem.writeFile(libraryPath, ''); - - const lsLibrary = await getService(libraryPath, rootUris, lsDocumentContext); - lsLibrary.getService(); - - const documentLibrary = new Document( - pathToUrl(libraryPath), - virtualSystem.readFile(libraryPath)! - ); - documentLibrary.openedByClient = true; - documentLibrary.update(' ', 0, documentLibrary.getTextLength()); - lsLibrary.updateSnapshot(documentLibrary); - - - const lsUsage = await getService(usagePath, rootUris, lsDocumentContext); - lsUsage.getService(); - }); - - it.skip('do not throw when ScriptKind changes referenced service is updated', async () => { - // testing this because the patch rely on ts implementation details - // and we want to be aware of the changes - - const dirPath = getRandomVirtualDirPath(testDir); - const { virtualSystem, lsDocumentContext, rootUris } = setup(); - const package1Path = path.join(dirPath, 'package1'); - const package2Path = path.join(dirPath, 'package2'); - - virtualSystem.writeFile( - path.join(package1Path, 'tsconfig.json'), - JSON.stringify({ - compilerOptions: { - allowJs: true - } - }) - ); - - virtualSystem.writeFile( - path.join(package2Path, 'tsconfig.json'), - JSON.stringify({ - compilerOptions: { - allowJs: true - } - }) - ); - - const usagePath = path.join(package1Path, 'usage.svelte'); - virtualSystem.writeFile( - usagePath, - '' - ); - - const libraryPath = path.join(package2Path, 'Library.svelte'); - virtualSystem.writeFile(libraryPath, ''); - - const lsLibrary = await getService(libraryPath, rootUris, lsDocumentContext); - lsLibrary.getService().getProgram(); - const lsUsage = await getService(usagePath, rootUris, lsDocumentContext); - lsUsage.getService(); - - const documentLibrary = new Document( - pathToUrl(libraryPath), - virtualSystem.readFile(libraryPath)! - ); - documentLibrary.openedByClient = true; - documentLibrary.update(' ', 0, documentLibrary.getTextLength()); - lsLibrary.updateSnapshot(documentLibrary); - - lsLibrary.getService(); - }); - function createReloadTester( docContext: LanguageServiceDocumentContext, testAfterReload: (reloadingConfigs: string[]) => Promise From d1e0de76db518ca09eeb915f04b915a86333b713 Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Tue, 24 Dec 2024 13:14:21 +0800 Subject: [PATCH 5/5] patch update as well --- .../src/plugins/typescript/service.ts | 65 ++++++++++++++----- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index fe2ff3a91..1fcb5eb4c 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -1388,6 +1388,54 @@ function getOrCreateDocumentRegistry( scriptKind?: ts.ScriptKind, sourceFileOptions?: ts.CreateSourceFileOptions | ts.ScriptTarget ) => { + ensureImpliedNodeFormat(compilationSettingsOrHost, fileName, sourceFileOptions); + + return acquireDocumentWithKey( + fileName, + path, + compilationSettingsOrHost, + key, + scriptSnapshot, + version, + scriptKind, + sourceFileOptions + ); + }; + + const updateDocumentWithKey = registry.updateDocumentWithKey; + registry.updateDocumentWithKey = ( + fileName: string, + path: ts.Path, + compilationSettingsOrHost: ts.CompilerOptions | ts.MinimalResolutionCacheHost, + key: ts.DocumentRegistryBucketKey, + scriptSnapshot: ts.IScriptSnapshot, + version: string, + scriptKind?: ts.ScriptKind, + sourceFileOptions?: ts.CreateSourceFileOptions | ts.ScriptTarget + ) => { + ensureImpliedNodeFormat(compilationSettingsOrHost, fileName, sourceFileOptions); + + return updateDocumentWithKey( + fileName, + path, + compilationSettingsOrHost, + key, + scriptSnapshot, + version, + scriptKind, + sourceFileOptions + ); + }; + + documentRegistries.set(key, registry); + + return registry; + + function ensureImpliedNodeFormat( + compilationSettingsOrHost: ts.CompilerOptions | ts.MinimalResolutionCacheHost, + fileName: string, + sourceFileOptions: ts.CreateSourceFileOptions | ts.ScriptTarget | undefined + ) { const compilationSettings = getCompilationSettings(compilationSettingsOrHost); const host: ts.MinimalResolutionCacheHost | undefined = compilationSettingsOrHost === compilationSettings @@ -1408,22 +1456,7 @@ function getOrCreateDocumentRegistry( sourceFileOptions.impliedNodeFormat = format; } - - return acquireDocumentWithKey( - fileName, - path, - compilationSettingsOrHost, - key, - scriptSnapshot, - version, - scriptKind, - sourceFileOptions - ); - }; - - documentRegistries.set(key, registry); - - return registry; + } function getCompilationSettings( settingsOrHost: ts.CompilerOptions | ts.MinimalResolutionCacheHost