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) detect invalid Svelte import paths #1448

Merged
merged 2 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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