revert exports map stuff from #2478

sveltejs · Aug 30, 2024 · d6bda3f · d6bda3f
1 parent ab81ce0
commit d6bda3f
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 62 deletions.
diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts
@@ -293,22 +293,6 @@ 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,

diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts
@@ -736,13 +736,6 @@ 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/.../test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/expectedv2.json b/.../test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/expectedv2.json
@@ -1,4 +1,21 @@
 [
+    {
+        "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.",

diff --git a/...ver/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/input.svelte b/...ver/test/plugins/typescript/features/diagnostics/fixtures/exports-map-svelte/input.svelte
@@ -1,5 +1,5 @@
 <script lang="ts">
-    import DefaultSvelteWithTS from 'package';
+    import DefaultSvelteWithTS from 'package'; // with https://github.com/sveltejs/language-tools/pull/2478 this would work; needs decision if we want that
     import SubWithDTS from 'package/x';
     import SubWithoutDTSAndNotTS from 'package/y';
 </script>

diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts
@@ -75,8 +75,7 @@ describe('service', () => {
             strict: true,
             module: ts.ModuleKind.ESNext,
             moduleResolution: ts.ModuleResolutionKind.Node10,
-            target: ts.ScriptTarget.ESNext,
-            customConditions: ['svelte']
+            target: ts.ScriptTarget.ESNext
         });
     });
 
@@ -186,8 +185,7 @@ describe('service', () => {
             moduleResolution: ts.ModuleResolutionKind.Node10,
             noEmit: true,
             skipLibCheck: true,
-            target: ts.ScriptTarget.ESNext,
-            customConditions: ['svelte']
+            target: ts.ScriptTarget.ESNext
         });
     });
 

diff --git 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<string, ts.ResolvedModuleFull | null>();
+    private cache = new Map<string, ts.ResolvedModuleFull>();
 
     /**
      * Tries to get a cached module.
      */
-    get(moduleName: string, containingFile: string): ts.ResolvedModuleFull | null | undefined {
+    get(moduleName: string, containingFile: string): ts.ResolvedModuleFull | undefined {
         return this.cache.get(this.getKey(moduleName, containingFile));
     }
 
@@ -28,14 +28,10 @@ class ModuleResolutionCache {
         containingFile: string,
         resolvedModule: ts.ResolvedModuleFull | undefined
     ) {
-        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.
+        if (!resolvedModule) {
             return;
         }
-        this.cache.set(this.getKey(moduleName, containingFile), resolvedModule ?? null);
+        this.cache.set(this.getKey(moduleName, containingFile), resolvedModule);
     }
 
     /**
@@ -46,7 +42,6 @@ class ModuleResolutionCache {
         resolvedModuleName = this.projectService.toCanonicalFileName(resolvedModuleName);
         this.cache.forEach((val, key) => {
             if (
-                val &&
                 this.projectService.toCanonicalFileName(val.resolvedFileName) === resolvedModuleName
             ) {
                 this.cache.delete(key);
@@ -146,9 +141,7 @@ export function patchModuleLoader(
         return resolved.map((tsResolvedModule, idx) => {
             const moduleName = moduleNames[idx];
             if (
-                // 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)) ||
+                !isSvelteFilePath(moduleName) ||
                 // corresponding .d.ts files take precedence over .svelte files
                 tsResolvedModule?.resolvedFileName.endsWith('.d.ts') ||
                 tsResolvedModule?.resolvedFileName.endsWith('.d.svelte.ts')
@@ -174,8 +167,7 @@ export function patchModuleLoader(
         const svelteResolvedModule = typescript.resolveModuleName(
             name,
             containingFile,
-            // customConditions makes the TS algorithm look at the "svelte" condition in exports maps
-            { ...compilerOptions, customConditions: ['svelte'] },
+            compilerOptions,
             svelteSys
             // don't set mode or else .svelte imports couldn't be resolved
         ).resolvedModule;
@@ -238,9 +230,7 @@ export function patchModuleLoader(
             const resolvedModule = tsResolvedModule.resolvedModule;
 
             if (
-                // 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)) ||
+                !isSvelteFilePath(moduleName) ||
                 // corresponding .d.ts files take precedence over .svelte files
                 resolvedModule?.resolvedFileName.endsWith('.d.ts') ||
                 resolvedModule?.resolvedFileName.endsWith('.d.svelte.ts')
@@ -260,29 +250,14 @@ export function patchModuleLoader(
         options: ts.CompilerOptions
     ) {
         const cachedModule = moduleCache.get(moduleName, containingFile);
-        if (typeof cachedModule === 'object') {
+        if (cachedModule) {
             return {
-                resolvedModule: cachedModule ?? undefined
+                resolvedModule: cachedModule
             };
         }
 
         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