From 1f8b5a04bfa3b4991d02eb8b8ed04c4229244de9 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 17:39:22 +0200 Subject: [PATCH 1/9] feat: resolve Svelte components using TS from exports map This change allows people to write export maps using only a `svelte` condition (and no `types` condition) and still have the types for their components resolved (i.e. the import is found) as long as they use TypeScript (i.e. have lang="ts" attribute) inside it. This should help people using monorepo setups with strong typings and not wanting to provide d.ts files alongside. This is achieved doing three adjustments: - add `customConditions: ['svelte']` to the compiler options, so that TypeScript's resolution algorithm takes it into account - ensure that Svelte files have a module kind of ESM, so that TypeScript's resolution algorithm goes into the right branches - deal with `.d.svelte.ts` files in the context of an exports map, because that's what TypeScript will try to resolve this to in the end This is also related to #1056 insofar that we align with TypeScript for this new capability: We don't resolve the file if it's a component not using TypeScript (i.e. not having the lang="ts" tag), similar to how TypeScript does not resolve .js files within node_modules --- .../src/plugins/typescript/module-loader.ts | 20 ++++++++++++++++++- .../src/plugins/typescript/service.ts | 7 +++++++ .../src/plugins/typescript/utils.ts | 8 +++++++- .../exports-map-svelte.only/expectedv2.json | 19 ++++++++++++++++++ .../exports-map-svelte.only/input.svelte | 9 +++++++++ .../exports-map-svelte.only/tsconfig.json | 7 +++++++ 6 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/expectedv2.json create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/input.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/tsconfig.json diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts index 915e50b26..5c440dbf1 100644 --- a/packages/language-server/src/plugins/typescript/module-loader.ts +++ b/packages/language-server/src/plugins/typescript/module-loader.ts @@ -104,10 +104,13 @@ class ImpliedNodeFormatResolver { return undefined; } - let mode = undefined; + let mode: ReturnType = undefined; if (sourceFile) { this.cacheImpliedNodeFormat(sourceFile, compilerOptions); mode = ts.getModeForResolutionAtIndex(sourceFile, importIdxInFile, compilerOptions); + if (!mode && isSvelteFilePath(importPath)) { + mode = ts.ModuleKind.ESNext; // necessary for TS' module resolution to go into the right branches + } } return mode; } @@ -293,6 +296,21 @@ export function createSvelteModuleLoader( const snapshot = getSnapshot(resolvedFileName); + // Align with TypeScript behavior: If the Svelte file is not using TypeScript, + // mark it as unresolved so that people need to provide a .d.ts file. + // For backwards compatibility we're not doing this for files from packages + // without an exports map, because that may break too many existing projects. + if ( + resolvedModule.isExternalLibraryImport && + resolvedModule.extension === '.d.svelte.ts' && // this tells us it's from an exports map + snapshot.scriptKind !== ts.ScriptKind.TS + ) { + return { + ...resolvedModuleWithFailedLookup, + resolvedModule: undefined + }; + } + const resolvedSvelteModule: ts.ResolvedModuleFull = { extension: getExtensionFromScriptKind(snapshot && snapshot.scriptKind), resolvedFileName, diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index 8f5720bb0..f94e8e6ff 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -736,6 +736,13 @@ async function createLanguageService( } } + // Necessary to be able to resolve export maps that only contain a "svelte" condition without an accompanying "types" condition + // https://www.typescriptlang.org/tsconfig/#customConditions + if (!compilerOptions.customConditions?.includes('svelte')) { + compilerOptions.customConditions = compilerOptions.customConditions ?? []; + compilerOptions.customConditions.push('svelte'); + } + const svelteConfigDiagnostics = checkSvelteInput(parsedConfig); if (svelteConfigDiagnostics.length > 0) { docContext.reportConfigError?.({ diff --git a/packages/language-server/src/plugins/typescript/utils.ts b/packages/language-server/src/plugins/typescript/utils.ts index 62fa6359d..9cb8b7f0f 100644 --- a/packages/language-server/src/plugins/typescript/utils.ts +++ b/packages/language-server/src/plugins/typescript/utils.ts @@ -74,7 +74,13 @@ export function isVirtualSvelteFilePath(filePath: string) { } export function toRealSvelteFilePath(filePath: string) { - return filePath.slice(0, -'.ts'.length); + filePath = filePath.slice(0, -'.ts'.length); + // When a .svelte file referenced inside an exports map of a package.json is tried to be resolved, + // TypeScript will probe for the file with a .d.svelte.ts extension. + if (filePath.endsWith('.d.svelte')) { + filePath = filePath.slice(0, -8) + 'svelte'; + } + return filePath; } export function toVirtualSvelteFilePath(filePath: string) { diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/expectedv2.json new file mode 100644 index 000000000..001dff42d --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/expectedv2.json @@ -0,0 +1,19 @@ +[ + { + "code": 2307, + "message": "Cannot find module 'package/y' or its corresponding type declarations.", + "range": { + "start": { + "character": 38, + "line": 3 + }, + "end": { + "character": 49, + "line": 3 + } + }, + "severity": 1, + "source": "ts", + "tags": [] + } +] diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/input.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/input.svelte new file mode 100644 index 000000000..3a6782e1b --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/input.svelte @@ -0,0 +1,9 @@ + + + + + diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/tsconfig.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/tsconfig.json new file mode 100644 index 000000000..d5e498207 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/tsconfig.json @@ -0,0 +1,7 @@ +{ + "compilerOptions": { + "module": "esnext", + "target": "esnext", + "moduleResolution": "Bundler" + } +} From cb7989c5def7a57b4d06b89f9637282cf159c021 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 17:52:46 +0200 Subject: [PATCH 2/9] oops --- .../expectedv2.json | 0 .../input.svelte | 0 .../tsconfig.json | 0 .../language-server/test/plugins/typescript/service.test.ts | 6 ++++-- 4 files changed, 4 insertions(+), 2 deletions(-) rename packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/{exports-map-svelte.only => exports-map-svelte}/expectedv2.json (100%) rename packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/{exports-map-svelte.only => exports-map-svelte}/input.svelte (100%) rename packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/{exports-map-svelte.only => exports-map-svelte}/tsconfig.json (100%) diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/expectedv2.json similarity index 100% rename from packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/expectedv2.json rename to packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/expectedv2.json diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/input.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/input.svelte similarity index 100% rename from packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/input.svelte rename to packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/input.svelte diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/tsconfig.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/tsconfig.json similarity index 100% rename from packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte.only/tsconfig.json rename to packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/tsconfig.json diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index 50c793c6f..266496dc7 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -75,7 +75,8 @@ describe('service', () => { strict: true, module: ts.ModuleKind.ESNext, moduleResolution: ts.ModuleResolutionKind.Node10, - target: ts.ScriptTarget.ESNext + target: ts.ScriptTarget.ESNext, + customConditions: ['svelte'] }); }); @@ -185,7 +186,8 @@ describe('service', () => { moduleResolution: ts.ModuleResolutionKind.Node10, noEmit: true, skipLibCheck: true, - target: ts.ScriptTarget.ESNext + target: ts.ScriptTarget.ESNext, + customConditions: ['svelte'] }); }); From 4a4645ba9f6aa0da3cc8ff410237aa6dce844fc6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 21:16:46 +0200 Subject: [PATCH 3/9] drive-by --- .../diagnostics/fixtures/bindings-two-way-check/expectedv2.json | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json @@ -0,0 +1 @@ +[] From c138c983e5c193b9d5839f20213025814d825191 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 21:17:23 +0200 Subject: [PATCH 4/9] drive-by --- .../bindings-two-way-check/expectedv2.json | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json index fe51488c7..f8c8cb449 100644 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json @@ -1 +1,18 @@ -[] +[ + { + "code": -1, + "message": "Unexpected token", + "range": { + "end": { + "character": 47, + "line": 12 + }, + "start": { + "character": 47, + "line": 12 + } + }, + "severity": 1, + "source": "ts" + } +] From dcc0507c7f214874b9d83155f2d64d3e5c61babb Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 29 Aug 2024 10:24:32 +0200 Subject: [PATCH 5/9] commit node_modules dir of test --- packages/language-server/.gitignore | 1 + .../node_modules/package/foo.svelte | 3 +++ .../node_modules/package/package.json | 16 ++++++++++++++++ .../node_modules/package/x-types.d.ts | 2 ++ .../node_modules/package/x.svelte | 3 +++ .../node_modules/package/y.svelte | 3 +++ 6 files changed, 28 insertions(+) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/foo.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/package.json create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x-types.d.ts create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/y.svelte diff --git a/packages/language-server/.gitignore b/packages/language-server/.gitignore index 5dbbc7fbb..02c400bc6 100644 --- a/packages/language-server/.gitignore +++ b/packages/language-server/.gitignore @@ -1,3 +1,4 @@ dist/ .vscode/ node_modules/ +!test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/ \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/foo.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/foo.svelte new file mode 100644 index 000000000..e29d29c54 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/foo.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/package.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/package.json new file mode 100644 index 000000000..422ee721e --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/package.json @@ -0,0 +1,16 @@ +{ + "name": "package", + "version": "1.0.0", + "exports": { + ".": { + "svelte": "./foo.svelte" + }, + "./x": { + "types": "./x-types.d.ts", + "svelte": "./x.svelte" + }, + "./y": { + "svelte": "./y.svelte" + } + } +} \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x-types.d.ts b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x-types.d.ts new file mode 100644 index 000000000..4d27ef5b0 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x-types.d.ts @@ -0,0 +1,2 @@ +declare const X: any; +export default X; \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x.svelte new file mode 100644 index 000000000..fb5dbfefa --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/x.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/y.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/y.svelte new file mode 100644 index 000000000..78f01008c --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/node_modules/package/y.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file From 72e815bace49c69696c69f34f06d552e28b88888 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 29 Aug 2024 23:20:11 +0200 Subject: [PATCH 6/9] use .d.svelte.ts for intercepting svelte files, which allows us to make `./foo.svelte` make .svelte query before .svelte.ts --- packages/language-server/package.json | 2 +- .../src/plugins/typescript/module-loader.ts | 3 --- .../src/plugins/typescript/svelte-sys.ts | 20 +++++++++++++++---- .../src/plugins/typescript/utils.ts | 16 ++++++--------- .../fixtures/import-precedence/a.svelte | 3 +++ .../fixtures/import-precedence/a.svelte.d.ts | 1 + .../fixtures/import-precedence/b.svelte | 3 +++ .../fixtures/import-precedence/b.svelte.ts | 1 + .../fixtures/import-precedence/c.d.svelte.ts | 1 + .../fixtures/import-precedence/c.svelte | 3 +++ .../fixtures/import-precedence/d.svelte | 3 +++ .../fixtures/import-precedence/d.svelte.js | 1 + .../import-precedence/expectedv2.json | 19 ++++++++++++++++++ .../fixtures/import-precedence/input.svelte | 15 ++++++++++++++ .../plugins/typescript/module-loader.test.ts | 4 ++-- .../plugins/typescript/svelte-sys.test.ts | 14 +++++++++---- 16 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte.d.ts create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte.ts create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.d.svelte.ts create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte.js create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/input.svelte diff --git a/packages/language-server/package.json b/packages/language-server/package.json index 8ee68b6ed..f3702c47b 100644 --- a/packages/language-server/package.json +++ b/packages/language-server/package.json @@ -10,7 +10,7 @@ "./bin/server.js": "./bin/server.js" }, "scripts": { - "test": "cross-env TS_NODE_TRANSPILE_ONLY=true mocha --require ts-node/register \"test/**/*.ts\" --exclude \"test/**/*.d.ts\"", + "test": "cross-env TS_NODE_TRANSPILE_ONLY=true mocha --require ts-node/register \"test/**/*.test.ts\"", "build": "tsc", "prepublishOnly": "npm run build", "watch": "tsc -w" diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts index 5c440dbf1..b0a3fa5f8 100644 --- a/packages/language-server/src/plugins/typescript/module-loader.ts +++ b/packages/language-server/src/plugins/typescript/module-loader.ts @@ -108,9 +108,6 @@ class ImpliedNodeFormatResolver { if (sourceFile) { this.cacheImpliedNodeFormat(sourceFile, compilerOptions); mode = ts.getModeForResolutionAtIndex(sourceFile, importIdxInFile, compilerOptions); - if (!mode && isSvelteFilePath(importPath)) { - mode = ts.ModuleKind.ESNext; // necessary for TS' module resolution to go into the right branches - } } return mode; } diff --git a/packages/language-server/src/plugins/typescript/svelte-sys.ts b/packages/language-server/src/plugins/typescript/svelte-sys.ts index a639b2a58..553bd17a3 100644 --- a/packages/language-server/src/plugins/typescript/svelte-sys.ts +++ b/packages/language-server/src/plugins/typescript/svelte-sys.ts @@ -11,6 +11,17 @@ export function createSvelteSys(tsSystem: ts.System) { function svelteFileExists(path: string) { if (isVirtualSvelteFilePath(path)) { const sveltePath = toRealSvelteFilePath(path); + + // First check if there's a `.svelte.d.ts` or `.d.svelte.ts` file, which should take precedence + const dtsPath = sveltePath.slice(0, -7) + '.svelte.d.ts'; + const dtsPathExists = fileExistsCache.get(dtsPath) ?? tsSystem.fileExists(dtsPath); + fileExistsCache.set(dtsPath, dtsPathExists); + if (dtsPathExists) return false; + + const svelteDtsPathExists = fileExistsCache.get(path) ?? tsSystem.fileExists(path); + fileExistsCache.set(path, svelteDtsPathExists); + if (svelteDtsPathExists) return false; + const sveltePathExists = fileExistsCache.get(sveltePath) ?? tsSystem.fileExists(sveltePath); fileExistsCache.set(sveltePath, sveltePathExists); @@ -33,10 +44,11 @@ export function createSvelteSys(tsSystem: ts.System) { svelteFileExists, getRealSveltePathIfExists, fileExists(path: string) { - // We need to check both .svelte and .svelte.ts/js because that's how Svelte 5 will likely mark files with runes in them + // We need to check if this is a virtual svelte file const sveltePathExists = svelteFileExists(path); - const exists = - sveltePathExists || (fileExistsCache.get(path) ?? tsSystem.fileExists(path)); + if (sveltePathExists) return true; + + const exists = fileExistsCache.get(path) ?? tsSystem.fileExists(path); fileExistsCache.set(path, exists); return exists; }, @@ -66,7 +78,7 @@ export function createSvelteSys(tsSystem: ts.System) { const realpath = tsSystem.realpath; svelteSys.realpath = function (path) { if (svelteFileExists(path)) { - return realpath(toRealSvelteFilePath(path)) + '.ts'; + return realpath(toRealSvelteFilePath(path)); } return realpath(path); }; diff --git a/packages/language-server/src/plugins/typescript/utils.ts b/packages/language-server/src/plugins/typescript/utils.ts index 9cb8b7f0f..a8a6ce1f6 100644 --- a/packages/language-server/src/plugins/typescript/utils.ts +++ b/packages/language-server/src/plugins/typescript/utils.ts @@ -70,21 +70,17 @@ export function isSvelteFilePath(filePath: string) { } export function isVirtualSvelteFilePath(filePath: string) { - return filePath.endsWith('.svelte.ts'); + return filePath.endsWith('.d.svelte.ts'); } export function toRealSvelteFilePath(filePath: string) { - filePath = filePath.slice(0, -'.ts'.length); - // When a .svelte file referenced inside an exports map of a package.json is tried to be resolved, - // TypeScript will probe for the file with a .d.svelte.ts extension. - if (filePath.endsWith('.d.svelte')) { - filePath = filePath.slice(0, -8) + 'svelte'; - } - return filePath; + return filePath.slice(0, -11 /* 'd.svelte.ts'.length */) + 'svelte'; } -export function toVirtualSvelteFilePath(filePath: string) { - return filePath.endsWith('.ts') ? filePath : filePath + '.ts'; +export function toVirtualSvelteFilePath(svelteFilePath: string) { + return isVirtualSvelteFilePath(svelteFilePath) + ? svelteFilePath + : svelteFilePath.slice(0, -6 /* 'svelte'.length */) + 'd.svelte.ts'; } export function ensureRealSvelteFilePath(filePath: string) { diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte new file mode 100644 index 000000000..1f6a1cbe4 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte.d.ts b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte.d.ts new file mode 100644 index 000000000..227cc1bb1 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/a.svelte.d.ts @@ -0,0 +1 @@ +export declare const a: boolean; diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte new file mode 100644 index 000000000..1f6a1cbe4 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte.ts b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte.ts new file mode 100644 index 000000000..1d9db10ef --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/b.svelte.ts @@ -0,0 +1 @@ +export const b = true; diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.d.svelte.ts b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.d.svelte.ts new file mode 100644 index 000000000..a12f971dd --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.d.svelte.ts @@ -0,0 +1 @@ +export declare const c: boolean; diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.svelte new file mode 100644 index 000000000..1f6a1cbe4 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte new file mode 100644 index 000000000..1f6a1cbe4 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte.js b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte.js new file mode 100644 index 000000000..ecee2a83c --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/d.svelte.js @@ -0,0 +1 @@ +export const d = true; diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json new file mode 100644 index 000000000..357f5c02f --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json @@ -0,0 +1,19 @@ +[ + { + "code": 6263, + "message": "Module './c.svelte' was resolved to 'c:/repos/svelte/language-tools/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.d.svelte.ts', but '--allowArbitraryExtensions' is not set.", + "range": { + "end": { + "character": 34, + "line": 4 + }, + "start": { + "character": 22, + "line": 4 + } + }, + "severity": 1, + "source": "ts", + "tags": [] + } +] diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/input.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/input.svelte new file mode 100644 index 000000000..9966c707e --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/input.svelte @@ -0,0 +1,15 @@ + + + + diff --git a/packages/language-server/test/plugins/typescript/module-loader.test.ts b/packages/language-server/test/plugins/typescript/module-loader.test.ts index 559eeb447..1c3561cc2 100644 --- a/packages/language-server/test/plugins/typescript/module-loader.test.ts +++ b/packages/language-server/test/plugins/typescript/module-loader.test.ts @@ -59,12 +59,12 @@ describe('createSvelteModuleLoader', () => { it('uses svelte script kind if resolved module is svelte file', async () => { const resolvedModule: ts.ResolvedModuleFull = { extension: ts.Extension.Ts, - resolvedFileName: 'filename.svelte.ts' + resolvedFileName: 'filename.d.svelte.ts' }; const { getSvelteSnapshotStub, moduleResolver, svelteSys } = setup(resolvedModule); svelteSys.getRealSveltePathIfExists = (filename: string) => - filename === 'filename.svelte.ts' ? 'filename.svelte' : filename; + filename === 'filename.d.svelte.ts' ? 'filename.svelte' : filename; const result = moduleResolver.resolveModuleNames( ['./normal.ts'], diff --git a/packages/language-server/test/plugins/typescript/svelte-sys.test.ts b/packages/language-server/test/plugins/typescript/svelte-sys.test.ts index 2da35db20..ba9a21d5a 100644 --- a/packages/language-server/test/plugins/typescript/svelte-sys.test.ts +++ b/packages/language-server/test/plugins/typescript/svelte-sys.test.ts @@ -29,18 +29,24 @@ describe('Svelte Sys', () => { } describe('#fileExists', () => { - it('should leave files with no .svelte.ts-ending as is', async () => { + it('should leave files with no .d.svelte.ts-ending as is', async () => { const { loader, fileExistsStub } = setupLoader(); loader.fileExists('../file.ts'); assert.strictEqual(fileExistsStub.getCall(0).args[0], '../file.ts'); }); - it('should convert .svelte.ts-endings', async () => { + it('should convert .d.svelte.ts-endings', async () => { const { loader, fileExistsStub } = setupLoader(); - loader.fileExists('../file.svelte.ts'); + fileExistsStub.onCall(0).returns(false); + fileExistsStub.onCall(1).returns(false); + fileExistsStub.onCall(2).returns(true); - assert.strictEqual(fileExistsStub.getCall(0).args[0], '../file.svelte'); + loader.fileExists('../file.d.svelte.ts'); + + assert.strictEqual(fileExistsStub.getCall(0).args[0], '../file.svelte.d.ts'); + assert.strictEqual(fileExistsStub.getCall(1).args[0], '../file.d.svelte.ts'); + assert.strictEqual(fileExistsStub.getCall(2).args[0], '../file.svelte'); }); }); }); From 9f75ed509e1f60e6ddbcb67df28c318653d845f3 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 29 Aug 2024 23:33:30 +0200 Subject: [PATCH 7/9] same for ts-plugin (major version bump) --- packages/typescript-plugin/package.json | 2 +- .../typescript-plugin/src/module-loader.ts | 52 ++++++++----------- packages/typescript-plugin/src/svelte-sys.ts | 2 +- packages/typescript-plugin/src/utils.ts | 10 +++- 4 files changed, 32 insertions(+), 34 deletions(-) diff --git a/packages/typescript-plugin/package.json b/packages/typescript-plugin/package.json index 6928186a4..766ac42c5 100644 --- a/packages/typescript-plugin/package.json +++ b/packages/typescript-plugin/package.json @@ -1,6 +1,6 @@ { "name": "typescript-svelte-plugin", - "version": "0.2.0", + "version": "0.3.0", "description": "A TypeScript Plugin providing Svelte intellisense", "main": "dist/src/index.js", "scripts": { diff --git a/packages/typescript-plugin/src/module-loader.ts b/packages/typescript-plugin/src/module-loader.ts index cc2003cba..76e3c1944 100644 --- a/packages/typescript-plugin/src/module-loader.ts +++ b/packages/typescript-plugin/src/module-loader.ts @@ -3,30 +3,7 @@ import { ConfigManager } from './config-manager'; import { Logger } from './logger'; import { SvelteSnapshotManager } from './svelte-snapshots'; import { createSvelteSys } from './svelte-sys'; -import { ensureRealSvelteFilePath, isVirtualSvelteFilePath } from './utils'; - -// TODO remove when we update to typescript 5.0 -declare module 'typescript/lib/tsserverlibrary' { - interface LanguageServiceHost { - /** @deprecated supply resolveModuleNameLiterals instead for resolution that can handle newer resolution modes like nodenext */ - resolveModuleNames?( - moduleNames: string[], - containingFile: string, - reusedNames: string[] | undefined, - redirectedReference: ts.ResolvedProjectReference | undefined, - options: ts.CompilerOptions, - containingSourceFile?: ts.SourceFile - ): (ts.ResolvedModule | undefined)[]; - resolveModuleNameLiterals?( - moduleLiterals: readonly ts.StringLiteralLike[], - containingFile: string, - redirectedReference: ts.ResolvedProjectReference | undefined, - options: ts.CompilerOptions, - containingSourceFile: ts.SourceFile, - reusedNames: readonly ts.StringLiteralLike[] | undefined - ): readonly ts.ResolvedModuleWithFailedLookupLocations[]; - } -} +import { ensureRealSvelteFilePath, isSvelteFilePath, isVirtualSvelteFilePath } from './utils'; /** * Caches resolved modules. @@ -161,12 +138,22 @@ export function patchModuleLoader( return resolved.map((tsResolvedModule, idx) => { const moduleName = moduleNames[idx]; - if (tsResolvedModule || !ensureRealSvelteFilePath(moduleName).endsWith('.svelte')) { + if ( + !isSvelteFilePath(moduleName) || + // corresponding .d.ts files take precedence over .svelte files + tsResolvedModule?.resolvedFileName.endsWith('.d.ts') || + tsResolvedModule?.resolvedFileName.endsWith('.d.svelte.ts') + ) { return tsResolvedModule; } - return resolveSvelteModuleNameFromCache(moduleName, containingFile, compilerOptions) - .resolvedModule; + const result = resolveSvelteModuleNameFromCache( + moduleName, + containingFile, + compilerOptions + ).resolvedModule; + // .svelte takes precedence over .svelte.ts etc + return result ?? tsResolvedModule; }); } @@ -238,14 +225,19 @@ export function patchModuleLoader( return resolved.map((tsResolvedModule, idx) => { const moduleName = moduleLiterals[idx].text; + if ( - tsResolvedModule.resolvedModule || - !ensureRealSvelteFilePath(moduleName).endsWith('.svelte') + !isSvelteFilePath(moduleName) || + // corresponding .d.ts files take precedence over .svelte files + tsResolvedModule?.resolvedModule?.resolvedFileName.endsWith('.d.ts') || + tsResolvedModule?.resolvedModule?.resolvedFileName.endsWith('.d.svelte.ts') ) { return tsResolvedModule; } - return resolveSvelteModuleNameFromCache(moduleName, containingFile, options); + const result = resolveSvelteModuleNameFromCache(moduleName, containingFile, options); + // .svelte takes precedence over .svelte.ts etc + return result ?? tsResolvedModule; }); } diff --git a/packages/typescript-plugin/src/svelte-sys.ts b/packages/typescript-plugin/src/svelte-sys.ts index d530e0b47..75b0a6f80 100644 --- a/packages/typescript-plugin/src/svelte-sys.ts +++ b/packages/typescript-plugin/src/svelte-sys.ts @@ -30,7 +30,7 @@ export function createSvelteSys(ts: _ts, logger: Logger) { const realpath = ts.sys.realpath; svelteSys.realpath = function (path) { if (isVirtualSvelteFilePath(path)) { - return realpath(toRealSvelteFilePath(path)) + '.ts'; + return realpath(toRealSvelteFilePath(path)); } return realpath(path); }; diff --git a/packages/typescript-plugin/src/utils.ts b/packages/typescript-plugin/src/utils.ts index d189f7ad1..1b14a6dbe 100644 --- a/packages/typescript-plugin/src/utils.ts +++ b/packages/typescript-plugin/src/utils.ts @@ -8,11 +8,17 @@ export function isSvelteFilePath(filePath: string) { } export function isVirtualSvelteFilePath(filePath: string) { - return filePath.endsWith('.svelte.ts'); + return filePath.endsWith('.d.svelte.ts'); } export function toRealSvelteFilePath(filePath: string) { - return filePath.slice(0, -'.ts'.length); + return filePath.slice(0, -11 /* 'd.svelte.ts'.length */) + 'svelte'; +} + +export function toVirtualSvelteFilePath(svelteFilePath: string) { + return isVirtualSvelteFilePath(svelteFilePath) + ? svelteFilePath + : svelteFilePath.slice(0, -6 /* 'svelte'.length */) + 'd.svelte.ts'; } export function ensureRealSvelteFilePath(filePath: string) { From 2f390ac9153d32b72f28929932662ca5f74f4a84 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 29 Aug 2024 23:41:40 +0200 Subject: [PATCH 8/9] fix test --- .../import-precedence/expectedv2.json | 20 +------------------ .../fixtures/import-precedence/tsconfig.json | 13 ++++++++++++ 2 files changed, 14 insertions(+), 19 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/tsconfig.json diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json index 357f5c02f..fe51488c7 100644 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/expectedv2.json @@ -1,19 +1 @@ -[ - { - "code": 6263, - "message": "Module './c.svelte' was resolved to 'c:/repos/svelte/language-tools/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/c.d.svelte.ts', but '--allowArbitraryExtensions' is not set.", - "range": { - "end": { - "character": 34, - "line": 4 - }, - "start": { - "character": 22, - "line": 4 - } - }, - "severity": 1, - "source": "ts", - "tags": [] - } -] +[] diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/tsconfig.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/tsconfig.json new file mode 100644 index 000000000..eef515315 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/import-precedence/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "strict": true, + "allowJs": true, + "target": "ESNext", + /** + This is actually not needed, but makes the tests faster + because TS does not look up other types. + */ + "types": ["svelte"], + "allowArbitraryExtensions": true // else .d.svelte.ts will be an error + } +} From ab81ce0e5c6364f8ecd516b6f98bc4b245913f34 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 30 Aug 2024 14:45:11 +0200 Subject: [PATCH 9/9] (WIP) package JSON exports support in TS --- .../src/plugins/typescript/module-loader.ts | 1 + .../typescript-plugin/src/module-loader.ts | 53 ++++++++++++++----- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts index b0a3fa5f8..86080dc15 100644 --- a/packages/language-server/src/plugins/typescript/module-loader.ts +++ b/packages/language-server/src/plugins/typescript/module-loader.ts @@ -299,6 +299,7 @@ export function createSvelteModuleLoader( // without an exports map, because that may break too many existing projects. if ( resolvedModule.isExternalLibraryImport && + // TODO check what happens if this resolves to a real .d.svelte.ts file resolvedModule.extension === '.d.svelte.ts' && // this tells us it's from an exports map snapshot.scriptKind !== ts.ScriptKind.TS ) { diff --git a/packages/typescript-plugin/src/module-loader.ts b/packages/typescript-plugin/src/module-loader.ts index 76e3c1944..3391b4439 100644 --- a/packages/typescript-plugin/src/module-loader.ts +++ b/packages/typescript-plugin/src/module-loader.ts @@ -11,12 +11,12 @@ import { ensureRealSvelteFilePath, isSvelteFilePath, isVirtualSvelteFilePath } f class ModuleResolutionCache { constructor(private readonly projectService: ts.server.ProjectService) {} - private cache = new Map(); + private cache = new Map(); /** * Tries to get a cached module. */ - get(moduleName: string, containingFile: string): ts.ResolvedModuleFull | undefined { + get(moduleName: string, containingFile: string): ts.ResolvedModuleFull | null | undefined { return this.cache.get(this.getKey(moduleName, containingFile)); } @@ -28,10 +28,14 @@ class ModuleResolutionCache { containingFile: string, resolvedModule: ts.ResolvedModuleFull | undefined ) { - if (!resolvedModule) { + if (!resolvedModule && moduleName[0] === '.') { + // We cache unresolved modules for non-relative imports, too, because it's very likely that they don't change + // and we don't want to resolve them every time. If they do change, the original resolution mode will notice + // most of the time, and the only time this would result in a stale cache entry is if a node_modules package + // is added with a "svelte" condition and no "types" condition, which is rare enough. return; } - this.cache.set(this.getKey(moduleName, containingFile), resolvedModule); + this.cache.set(this.getKey(moduleName, containingFile), resolvedModule ?? null); } /** @@ -42,6 +46,7 @@ class ModuleResolutionCache { resolvedModuleName = this.projectService.toCanonicalFileName(resolvedModuleName); this.cache.forEach((val, key) => { if ( + val && this.projectService.toCanonicalFileName(val.resolvedFileName) === resolvedModuleName ) { this.cache.delete(key); @@ -87,6 +92,8 @@ export function patchModuleLoader( if (lsHost.resolveModuleNameLiterals) { lsHost.resolveModuleNameLiterals = resolveModuleNameLiterals; } else { + // TODO do we need to keep this around? We're requiring 5.0 now, so TS doesn't need it, + // but would this break when other TS plugins are used and we no longer provide it? lsHost.resolveModuleNames = resolveModuleNames; } @@ -139,7 +146,9 @@ export function patchModuleLoader( return resolved.map((tsResolvedModule, idx) => { const moduleName = moduleNames[idx]; if ( - !isSvelteFilePath(moduleName) || + // Only recheck relative Svelte imports or unresolved non-relative paths (which hint at node_modules, + // where an exports map with "svelte" but not "types" could be present) + (!isSvelteFilePath(moduleName) && (moduleName[0] === '.' || tsResolvedModule)) || // corresponding .d.ts files take precedence over .svelte files tsResolvedModule?.resolvedFileName.endsWith('.d.ts') || tsResolvedModule?.resolvedFileName.endsWith('.d.svelte.ts') @@ -165,7 +174,8 @@ export function patchModuleLoader( const svelteResolvedModule = typescript.resolveModuleName( name, containingFile, - compilerOptions, + // customConditions makes the TS algorithm look at the "svelte" condition in exports maps + { ...compilerOptions, customConditions: ['svelte'] }, svelteSys // don't set mode or else .svelte imports couldn't be resolved ).resolvedModule; @@ -225,19 +235,22 @@ export function patchModuleLoader( return resolved.map((tsResolvedModule, idx) => { const moduleName = moduleLiterals[idx].text; + const resolvedModule = tsResolvedModule.resolvedModule; if ( - !isSvelteFilePath(moduleName) || + // Only recheck relative Svelte imports or unresolved non-relative paths (which hint at node_modules, + // where an exports map with "svelte" but not "types" could be present) + (!isSvelteFilePath(moduleName) && (moduleName[0] === '.' || resolvedModule)) || // corresponding .d.ts files take precedence over .svelte files - tsResolvedModule?.resolvedModule?.resolvedFileName.endsWith('.d.ts') || - tsResolvedModule?.resolvedModule?.resolvedFileName.endsWith('.d.svelte.ts') + resolvedModule?.resolvedFileName.endsWith('.d.ts') || + resolvedModule?.resolvedFileName.endsWith('.d.svelte.ts') ) { return tsResolvedModule; } const result = resolveSvelteModuleNameFromCache(moduleName, containingFile, options); // .svelte takes precedence over .svelte.ts etc - return result ?? tsResolvedModule; + return result.resolvedModule ? result : tsResolvedModule; }); } @@ -247,13 +260,29 @@ export function patchModuleLoader( options: ts.CompilerOptions ) { const cachedModule = moduleCache.get(moduleName, containingFile); - if (cachedModule) { + if (typeof cachedModule === 'object') { return { - resolvedModule: cachedModule + resolvedModule: cachedModule ?? undefined }; } const resolvedModule = resolveSvelteModuleName(moduleName, containingFile, options); + + // Align with TypeScript behavior: If the Svelte file is not using TypeScript, + // mark it as unresolved so that people need to provide a .d.ts file. + // For backwards compatibility we're not doing this for files from packages + // without an exports map, because that may break too many existing projects. + if ( + resolvedModule?.isExternalLibraryImport && // TODO how to check this is not from a non-exports map? + // TODO check what happens if this resolves to a real .d.svelte.ts file + resolvedModule.extension === '.ts' // this tells us it's from an exports map + ) { + moduleCache.set(moduleName, containingFile, undefined); + return { + resolvedModule: undefined + }; + } + moduleCache.set(moduleName, containingFile, resolvedModule); return { resolvedModule: resolvedModule