Skip to content

Commit

Permalink
(feat) detect invalid Svelte import paths (#1448)
Browse files Browse the repository at this point in the history
Detect that an import of a .svelte file is pointing to a non-existent file by removing the ambient module definitions in svelte and svelte2tsx from the eyes of TS
#1444
  • Loading branch information
dummdidumm authored Apr 14, 2022
1 parent d4c2fc1 commit 1415196
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,30 @@ export namespace DocumentSnapshot {
* @param options options that apply in case it's a svelte file
*/
export function fromNonSvelteFilePath(filePath: string) {
const originalText = ts.sys.readFile(filePath) ?? '';
let originalText = '';

// The following (very hacky) code makes sure that the ambient module definitions
// that tell TS "every import ending with .svelte is a valid module" are removed.
// They exist in svelte2tsx and svelte to make sure that people don't
// get errors in their TS files when importing Svelte files and not using our TS plugin.
// If someone wants to get back the behavior they can add an ambient module definition
// on their own.
const normalizedPath = filePath.replace(/\\/g, '/');
if (!normalizedPath.endsWith('node_modules/svelte/types/runtime/ambient.d.ts')) {
originalText = ts.sys.readFile(filePath) || '';
}
if (normalizedPath.endsWith('svelte2tsx/svelte-shims.d.ts')) {
// If not present, the LS uses an older version of svelte2tsx
if (originalText.includes('// -- start svelte-ls-remove --')) {
originalText =
originalText.substring(
0,
originalText.indexOf('// -- start svelte-ls-remove --')
) +
originalText.substring(originalText.indexOf('// -- end svelte-ls-remove --'));
}
}

return new JSOrTSDocumentSnapshot(INITIAL_VERSION, filePath, originalText);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
[
{
"range": { "start": { "line": 9, "character": 7 }, "end": { "line": 9, "character": 16 } },
"range": {
"start": { "line": 10, "character": 7 },
"end": { "line": 10, "character": 16 }
},
"severity": 1,
"source": "ts",
"message": "Type 'boolean' is not assignable to type 'string'.",
Expand All @@ -9,8 +12,8 @@
},
{
"range": {
"start": { "line": 10, "character": 43 },
"end": { "line": 10, "character": 54 }
"start": { "line": 11, "character": 43 },
"end": { "line": 11, "character": 54 }
},
"severity": 1,
"source": "ts",
Expand All @@ -19,7 +22,7 @@
"tags": []
},
{
"range": { "start": { "line": 11, "character": 1 }, "end": { "line": 11, "character": 6 } },
"range": { "start": { "line": 12, "character": 1 }, "end": { "line": 12, "character": 6 } },
"severity": 1,
"source": "ts",
"message": "Type '{}' is not assignable to type 'IntrinsicAttributes & { exported1: string; exported2?: string | undefined; }'.\n Property 'exported1' is missing in type '{}' but required in type '{ exported1: string; exported2?: string | undefined; }'.",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
[
{
"range": { "start": { "line": 9, "character": 7 }, "end": { "line": 9, "character": 16 } },
"range": {
"start": { "line": 10, "character": 7 },
"end": { "line": 10, "character": 16 }
},
"severity": 1,
"source": "ts",
"message": "Type 'boolean' is not assignable to type 'string'.",
Expand All @@ -9,8 +12,8 @@
},
{
"range": {
"start": { "line": 10, "character": 43 },
"end": { "line": 10, "character": 61 }
"start": { "line": 11, "character": 43 },
"end": { "line": 11, "character": 61 }
},
"severity": 1,
"source": "ts",
Expand All @@ -19,7 +22,7 @@
"tags": []
},
{
"range": { "start": { "line": 11, "character": 1 }, "end": { "line": 11, "character": 6 } },
"range": { "start": { "line": 12, "character": 1 }, "end": { "line": 12, "character": 6 } },
"severity": 1,
"source": "ts",
"message": "Property 'exported1' is missing in type '{}' but required in type '{ exported1: string; exported2?: string | undefined; }'.",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<script lang="ts">
import Props from '../$$props-valid/input.svelte';
// @ts-expect-error
import PropsInvalid3 from './$$props-invalid3.svelte';
</script>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[
{
"range": { "start": { "line": 1, "character": 28 }, "end": { "line": 1, "character": 51 } },
"severity": 1,
"source": "ts",
"message": "Cannot find module './doesnt-exist.svelte' or its corresponding type declarations.",
"code": 2307,
"tags": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[
{
"range": { "start": { "line": 1, "character": 28 }, "end": { "line": 1, "character": 51 } },
"severity": 1,
"source": "ts",
"message": "Cannot find module './doesnt-exist.svelte' or its corresponding type declarations.",
"code": 2307,
"tags": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script lang="ts">
import InvalidImport from './doesnt-exist.svelte';
import ValidImport from './valid-import.svelte';
InvalidImport;ValidImport;
</script>
2 changes: 2 additions & 0 deletions packages/svelte2tsx/svelte-shims.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
// are loaded and their declarations conflict each other
// See https://github.com/sveltejs/language-tools/issues/1059 for an example bug that stems from it

// -- start svelte-ls-remove --
declare module '*.svelte' {
export default Svelte2TsxComponent
}
// -- end svelte-ls-remove --

declare class Svelte2TsxComponent<
Props extends {} = {},
Expand Down
23 changes: 22 additions & 1 deletion packages/typescript-plugin/src/svelte-snapshots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,28 @@ export class SvelteSnapshotManager {
private patchProjectServiceReadFile() {
const readFile = this.projectService.host.readFile;
this.projectService.host.readFile = (path: string) => {
if (isSvelteFilePath(path) && this.configManager.getConfig().enable) {
// The following (very hacky) first two checks make sure that the ambient module definitions
// that tell TS "every import ending with .svelte is a valid module" are removed.
// They exist in svelte2tsx and svelte to make sure that people don't
// get errors in their TS files when importing Svelte files and not using our TS plugin.
// If someone wants to get back the behavior they can add an ambient module definition
// on their own.
const normalizedPath = path.replace(/\\/g, '/');
if (normalizedPath.endsWith('node_modules/svelte/types/runtime/ambient.d.ts')) {
return '';
} else if (normalizedPath.endsWith('svelte2tsx/svelte-shims.d.ts')) {
let originalText = readFile(path) || '';
if (!originalText.includes('// -- start svelte-ls-remove --')) {
return originalText; // uses an older version of svelte2tsx
}
originalText =
originalText.substring(
0,
originalText.indexOf('// -- start svelte-ls-remove --')
) +
originalText.substring(originalText.indexOf('// -- end svelte-ls-remove --'));
return originalText;
} else if (isSvelteFilePath(path) && this.configManager.getConfig().enable) {
this.logger.debug('Read Svelte file:', path);
const svelteCode = readFile(path) || '';
try {
Expand Down

0 comments on commit 1415196

Please sign in to comment.