From ade110b79e104221e2e6062faccebaf73edc852e Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 11:37:49 +0200 Subject: [PATCH 1/4] feat: better type checking for bindings in Svelte 5 #1392 --- .../svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts | 4 ++++ .../src/htmlxtojsx_v2/nodes/InlineComponent.ts | 1 + packages/svelte2tsx/svelte-shims-v4.d.ts | 13 +++++++++++++ 3 files changed, 18 insertions(+) diff --git a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts index 5bb17cca7..4d2390b74 100644 --- a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts +++ b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts @@ -131,6 +131,10 @@ export function handleBinding( if (isSvelte5Plus && element instanceof InlineComponent) { // To check if property is actually bindable element.appendToStartEnd([`${element.name}.$$bindings = '${attr.name}';`]); + // To check if the binding is also assigned to the variable + element.appendToStartEnd([ + `${expressionStr} = __sveltets_binding_value(${element.originalName}, '${attr.name}');` + ]); } if (element instanceof Element) { diff --git a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts index b1bab058d..7459f227e 100644 --- a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts +++ b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts @@ -39,6 +39,7 @@ export class InlineComponent { private startTagEnd: number; private isSelfclosing: boolean; public child?: any; + public originalName = this.node.name; // Add const $$xxx = ... only if the variable name is actually used // in order to prevent "$$xxx is defined but never used" TS hints diff --git a/packages/svelte2tsx/svelte-shims-v4.d.ts b/packages/svelte2tsx/svelte-shims-v4.d.ts index 87e365798..8589dc017 100644 --- a/packages/svelte2tsx/svelte-shims-v4.d.ts +++ b/packages/svelte2tsx/svelte-shims-v4.d.ts @@ -254,3 +254,16 @@ declare function __sveltets_2_isomorphic_component< declare function __sveltets_2_isomorphic_component_slots< Props extends Record, Events extends Record, Slots extends Record, Exports extends Record, Bindings extends string >(klass: {props: Props, events: Events, slots: Slots, exports?: Exports, bindings?: Bindings }): __sveltets_2_IsomorphicComponent<__sveltets_2_PropsWithChildren, Events, Slots, Exports, Bindings>; + +type __sveltets_NonUndefined = T extends undefined ? never : T; + +declare function __sveltets_binding_value< + // @ts-ignore this is only used for Svelte 5, which knows about the Component type + Comp extends typeof import('svelte').Component, + Key extends string +>(comp: Comp, key: Key): Key extends keyof import('svelte').ComponentProps ? + // bail on unknown because it hints at a generic type which we can't properly resolve here + // remove undefined because optional properties have it, and would result in false positives + unknown extends import('svelte').ComponentProps[Key] ? any : __sveltets_NonUndefined[Key]> : any; +// Overload to ensure typings that only use old SvelteComponent class are gracefully handled +declare function __sveltets_binding_value>(comp: Comp, key: string): any From e0d60f11f3266a5aeb762856d626b51b4105bc80 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 16:09:09 +0200 Subject: [PATCH 2/4] tests + fixes --- .../bindings-two-way-check/Legacy.svelte | 7 ++++ .../bindings-two-way-check/Runes.svelte | 6 ++++ .../expected_svelte_5.json | 36 +++++++++++++++++++ .../bindings-two-way-check/input.svelte | 15 ++++++++ packages/svelte2tsx/repl/index.svelte | 12 +++---- .../src/htmlxtojsx_v2/nodes/Binding.ts | 10 +++--- packages/svelte2tsx/svelte-shims-v4.d.ts | 4 +-- .../samples/binding-bare/expected-svelte5.js | 4 +-- .../samples/binding/expected-svelte5.js | 8 ++--- .../editing-binding/expected-svelte5.js | 2 +- 10 files changed, 85 insertions(+), 19 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Legacy.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Runes.svelte create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Legacy.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Legacy.svelte new file mode 100644 index 000000000..18db75ea2 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Legacy.svelte @@ -0,0 +1,7 @@ + + +{legacy1} +{legacy2} \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Runes.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Runes.svelte new file mode 100644 index 000000000..299be4d66 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Runes.svelte @@ -0,0 +1,6 @@ + + +{runes1} +{runes2} \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json new file mode 100644 index 000000000..4fb544181 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json @@ -0,0 +1,36 @@ +[ + { + "code": 2322, + "message": "Type 'string | number' is not assignable to type 'string'.\n Type 'number' is not assignable to type 'string'.", + "range": { + "end": { + "character": 28, + "line": 13 + }, + "start": { + "character": 28, + "line": 13 + } + }, + "severity": 1, + "source": "ts", + "tags": [] + }, + { + "code": 2322, + "message": "Type 'string | number' is not assignable to type 'string'.\n Type 'number' is not assignable to type 'string'.", + "range": { + "end": { + "character": 45, + "line": 14 + }, + "start": { + "character": 45, + "line": 14 + } + }, + "severity": 1, + "source": "ts", + "tags": [] + } +] diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte new file mode 100644 index 000000000..d39c5f451 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte @@ -0,0 +1,15 @@ + + + + + + + + + diff --git a/packages/svelte2tsx/repl/index.svelte b/packages/svelte2tsx/repl/index.svelte index 49517cc72..bf8061fb0 100644 --- a/packages/svelte2tsx/repl/index.svelte +++ b/packages/svelte2tsx/repl/index.svelte @@ -1,7 +1,7 @@ - - -{#if value} - -{/if} + + \ No newline at end of file diff --git a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts index 4d2390b74..555269b61 100644 --- a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts +++ b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts @@ -131,10 +131,12 @@ export function handleBinding( if (isSvelte5Plus && element instanceof InlineComponent) { // To check if property is actually bindable element.appendToStartEnd([`${element.name}.$$bindings = '${attr.name}';`]); - // To check if the binding is also assigned to the variable - element.appendToStartEnd([ - `${expressionStr} = __sveltets_binding_value(${element.originalName}, '${attr.name}');` - ]); + // To check if the binding is also assigned to the variable (only works when there's no assertion, we can't transform that) + if (!isTypescriptNode(attr.expression)) { + element.appendToStartEnd([ + `${expressionStr} = __sveltets_binding_value(${element.originalName}, '${attr.name}');` + ]); + } } if (element instanceof Element) { diff --git a/packages/svelte2tsx/svelte-shims-v4.d.ts b/packages/svelte2tsx/svelte-shims-v4.d.ts index 8589dc017..5227cb041 100644 --- a/packages/svelte2tsx/svelte-shims-v4.d.ts +++ b/packages/svelte2tsx/svelte-shims-v4.d.ts @@ -265,5 +265,5 @@ declare function __sveltets_binding_value< // bail on unknown because it hints at a generic type which we can't properly resolve here // remove undefined because optional properties have it, and would result in false positives unknown extends import('svelte').ComponentProps[Key] ? any : __sveltets_NonUndefined[Key]> : any; -// Overload to ensure typings that only use old SvelteComponent class are gracefully handled -declare function __sveltets_binding_value>(comp: Comp, key: string): any +// Overload to ensure typings that only use old SvelteComponent class or something invalid are gracefully handled +declare function __sveltets_binding_value(comp: any, key: string): any diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/binding-bare/expected-svelte5.js b/packages/svelte2tsx/test/htmlx2jsx/samples/binding-bare/expected-svelte5.js index bb934f658..43291dbae 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/binding-bare/expected-svelte5.js +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/binding-bare/expected-svelte5.js @@ -1,5 +1,5 @@ { svelteHTML.createElement("input", { "type":`text`,"bind:value":value,});/*Ωignore_startΩ*/() => value = __sveltets_2_any(null);/*Ωignore_endΩ*/} { svelteHTML.createElement("input", { "type":`checkbox`,"bind:checked":checked,});/*Ωignore_startΩ*/() => checked = __sveltets_2_any(null);/*Ωignore_endΩ*/} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value,}});/*Ωignore_startΩ*/() => value = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`checkbox`,checked,}});/*Ωignore_startΩ*/() => checked = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'checked';} \ No newline at end of file + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value,}});/*Ωignore_startΩ*/() => value = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';value = __sveltets_binding_value(Input, 'value');} + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`checkbox`,checked,}});/*Ωignore_startΩ*/() => checked = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'checked';checked = __sveltets_binding_value(Input, 'checked');} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/binding/expected-svelte5.js b/packages/svelte2tsx/test/htmlx2jsx/samples/binding/expected-svelte5.js index 6c881fec9..355227003 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/binding/expected-svelte5.js +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/binding/expected-svelte5.js @@ -2,7 +2,7 @@ { svelteHTML.createElement("input", { "type":`text`,"bind:value":test,});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/} { svelteHTML.createElement("input", { "type":`text`,"bind:value":test,});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value'; Input} \ No newline at end of file + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';test = __sveltets_binding_value(Input, 'value');} + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';test = __sveltets_binding_value(Input, 'value');} + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';test = __sveltets_binding_value(Input, 'value');} + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { "type":`text`,value:test,}});/*Ωignore_startΩ*/() => test = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';test = __sveltets_binding_value(Input, 'value'); Input} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/editing-binding/expected-svelte5.js b/packages/svelte2tsx/test/htmlx2jsx/samples/editing-binding/expected-svelte5.js index 7bb207fea..31e8b2747 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/editing-binding/expected-svelte5.js +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/editing-binding/expected-svelte5.js @@ -1,3 +1,3 @@ { svelteHTML.createElement("input", { });obj = __sveltets_2_any(null);} { svelteHTML.createElement("input", { "bind:value":obj.,});/*Ωignore_startΩ*/() => obj = __sveltets_2_any(null);/*Ωignore_endΩ*/} - { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { value:obj.,}});/*Ωignore_startΩ*/() => obj = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';} \ No newline at end of file + { const $$_tupnI0C = __sveltets_2_ensureComponent(Input); const $$_tupnI0 = new $$_tupnI0C({ target: __sveltets_2_any(), props: { value:obj.,}});/*Ωignore_startΩ*/() => obj = __sveltets_2_any(null);/*Ωignore_endΩ*/$$_tupnI0.$$bindings = 'value';obj = __sveltets_binding_value(Input, 'value');} \ No newline at end of file From 8b8d89f3ae8ece78613bdb97b6c3831a94d0242e Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 16:51:10 +0200 Subject: [PATCH 3/4] fix --- .../src/plugins/typescript/features/RenameProvider.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts index 3b9caf86e..8340478c3 100644 --- a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts @@ -463,8 +463,16 @@ export class RenameProviderImpl implements RenameProvider { const mappedLocations = await Promise.all( renameLocations.map(async (loc) => { const snapshot = await snapshots.retrieve(loc.fileName); + const text = snapshot.getFullText(); + const end = loc.textSpan.start + loc.textSpan.length; - if (!isTextSpanInGeneratedCode(snapshot.getFullText(), loc.textSpan)) { + if ( + !(snapshot instanceof SvelteDocumentSnapshot) || + (!isTextSpanInGeneratedCode(text, loc.textSpan) && + // prevent generated code for bindings from being renamed + // (it's not inside a generate comment because diagnostics should show up) + text.slice(end + 3, end + 27) !== '__sveltets_binding_value') + ) { return { ...loc, range: this.mapRangeToOriginal(snapshot, loc.textSpan), From 9a587b140b36abf1e0e3ac853f09f2e1185f78e8 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 28 Aug 2024 17:16:04 +0200 Subject: [PATCH 4/4] generics --- .../fixtures/bindings-two-way-check/RunesGeneric.svelte | 6 ++++++ .../bindings-two-way-check/expected_svelte_5.json | 8 ++++---- .../fixtures/bindings-two-way-check/input.svelte | 4 ++++ 3 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/RunesGeneric.svelte diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/RunesGeneric.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/RunesGeneric.svelte new file mode 100644 index 000000000..6accfc4b3 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/RunesGeneric.svelte @@ -0,0 +1,6 @@ + + +{foo} +{bar} \ No newline at end of file diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json index 4fb544181..6b1d57c83 100644 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json @@ -5,11 +5,11 @@ "range": { "end": { "character": 28, - "line": 13 + "line": 17 }, "start": { "character": 28, - "line": 13 + "line": 17 } }, "severity": 1, @@ -22,11 +22,11 @@ "range": { "end": { "character": 45, - "line": 14 + "line": 18 }, "start": { "character": 45, - "line": 14 + "line": 18 } }, "severity": 1, diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte index d39c5f451..49020f784 100644 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte @@ -1,14 +1,18 @@ + +