Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: resolve Svelte components using TS from exports map #2478

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
16 changes: 16 additions & 0 deletions packages/language-server/src/plugins/typescript/module-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,22 @@ 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 &&
// 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
) {
return {
...resolvedModuleWithFailedLookup,
resolvedModule: undefined
};
}

const resolvedSvelteModule: ts.ResolvedModuleFull = {
extension: getExtensionFromScriptKind(snapshot && snapshot.scriptKind),
resolvedFileName,
Expand Down
7 changes: 7 additions & 0 deletions packages/language-server/src/plugins/typescript/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?.({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,4 @@
[
{
"code": 2307,
"message": "Cannot find module 'package' or its corresponding type declarations.",
"range": {
"end": {
"character": 45,
"line": 1
},
"start": {
"character": 36,
"line": 1
}
},
"severity": 1,
"source": "ts",
"tags": []
},
{
"code": 2307,
"message": "Cannot find module 'package/y' or its corresponding type declarations.",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script lang="ts">
import DefaultSvelteWithTS from 'package'; // with https://github.com/sveltejs/language-tools/pull/2478 this would work; needs decision if we want that
import DefaultSvelteWithTS from 'package';
import SubWithDTS from 'package/x';
import SubWithoutDTSAndNotTS from 'package/y';
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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']
});
});

Expand Down Expand Up @@ -185,7 +186,8 @@ describe('service', () => {
moduleResolution: ts.ModuleResolutionKind.Node10,
noEmit: true,
skipLibCheck: true,
target: ts.ScriptTarget.ESNext
target: ts.ScriptTarget.ESNext,
customConditions: ['svelte']
});
});

Expand Down
43 changes: 34 additions & 9 deletions packages/typescript-plugin/src/module-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import { ensureRealSvelteFilePath, isSvelteFilePath, isVirtualSvelteFilePath } f
class ModuleResolutionCache {
constructor(private readonly projectService: ts.server.ProjectService) {}

private cache = new Map<string, ts.ResolvedModuleFull>();
private cache = new Map<string, ts.ResolvedModuleFull | null>();

/**
* 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));
}

Expand All @@ -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);
}

/**
Expand All @@ -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);
Expand Down Expand Up @@ -141,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')
Expand All @@ -167,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;
Expand Down Expand Up @@ -230,7 +238,9 @@ export function patchModuleLoader(
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
resolvedModule?.resolvedFileName.endsWith('.d.ts') ||
resolvedModule?.resolvedFileName.endsWith('.d.svelte.ts')
Expand All @@ -250,14 +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
Expand Down