From 81019d91c38effe61c2ac4b8e8f830d8a6d1f148 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Thu, 19 Sep 2024 15:06:10 +0200 Subject: [PATCH] fix: revert additional two-way-binding checks (#2508) This reverts commit 8c080cf36286984439b468e089e6e4eaacd4230e. This reverts #2477. Sadly, the idea didn't work out, as shown by two opened bug reports: - #2506: A type union can be narrowed on the input, but not on the way back out - #2494: A generic nested within a bound value is not properly resolved and not falling back to `any` (in #2477 we thought of the generic case, but only for when the generic is the whole value type, not when it's nested) For these reasons I don't see a way to properly implement #1392 at the moment. --- .../typescript/features/RenameProvider.ts | 10 +----- .../bindings-two-way-check/Legacy.svelte | 7 ---- .../bindings-two-way-check/Runes.svelte | 6 ---- .../RunesGeneric.svelte | 6 ---- .../expected_svelte_5.json | 36 ------------------- .../bindings-two-way-check/expectedv2.json | 18 ---------- .../bindings-two-way-check/input.svelte | 19 ---------- packages/svelte2tsx/repl/index.svelte | 12 +++---- .../src/htmlxtojsx_v2/nodes/Binding.ts | 6 ---- .../htmlxtojsx_v2/nodes/InlineComponent.ts | 1 - packages/svelte2tsx/svelte-shims-v4.d.ts | 13 ------- .../samples/binding-bare/expected-svelte5.js | 4 +-- .../samples/binding/expected-svelte5.js | 8 ++--- .../editing-binding/expected-svelte5.js | 2 +- 14 files changed, 14 insertions(+), 134 deletions(-) delete mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Legacy.svelte delete mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Runes.svelte delete mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/RunesGeneric.svelte delete mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json delete mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json delete mode 100644 packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte diff --git a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts index 8340478c3..3b9caf86e 100644 --- a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts @@ -463,16 +463,8 @@ 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 ( - !(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') - ) { + if (!isTextSpanInGeneratedCode(snapshot.getFullText(), loc.textSpan)) { return { ...loc, range: this.mapRangeToOriginal(snapshot, loc.textSpan), 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 deleted file mode 100644 index 18db75ea2..000000000 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Legacy.svelte +++ /dev/null @@ -1,7 +0,0 @@ - - -{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 deleted file mode 100644 index 299be4d66..000000000 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/Runes.svelte +++ /dev/null @@ -1,6 +0,0 @@ - - -{runes1} -{runes2} \ No newline at end of file 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 deleted file mode 100644 index 6accfc4b3..000000000 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/RunesGeneric.svelte +++ /dev/null @@ -1,6 +0,0 @@ - - -{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 deleted file mode 100644 index 6b1d57c83..000000000 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expected_svelte_5.json +++ /dev/null @@ -1,36 +0,0 @@ -[ - { - "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": 17 - }, - "start": { - "character": 28, - "line": 17 - } - }, - "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": 18 - }, - "start": { - "character": 45, - "line": 18 - } - }, - "severity": 1, - "source": "ts", - "tags": [] - } -] diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json b/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json deleted file mode 100644 index f8c8cb449..000000000 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/expectedv2.json +++ /dev/null @@ -1,18 +0,0 @@ -[ - { - "code": -1, - "message": "Unexpected token", - "range": { - "end": { - "character": 47, - "line": 12 - }, - "start": { - "character": 47, - "line": 12 - } - }, - "severity": 1, - "source": "ts" - } -] 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 deleted file mode 100644 index 49020f784..000000000 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/fixtures/bindings-two-way-check/input.svelte +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - - - - diff --git a/packages/svelte2tsx/repl/index.svelte b/packages/svelte2tsx/repl/index.svelte index bf8061fb0..49517cc72 100644 --- a/packages/svelte2tsx/repl/index.svelte +++ b/packages/svelte2tsx/repl/index.svelte @@ -1,7 +1,7 @@ - - - \ No newline at end of file + +{#if value} + +{/if} diff --git a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts index 555269b61..5bb17cca7 100644 --- a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts +++ b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/Binding.ts @@ -131,12 +131,6 @@ 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 (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/src/htmlxtojsx_v2/nodes/InlineComponent.ts b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts index 7459f227e..b1bab058d 100644 --- a/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts +++ b/packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts @@ -39,7 +39,6 @@ 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 f01cf6d18..70e6df540 100644 --- a/packages/svelte2tsx/svelte-shims-v4.d.ts +++ b/packages/svelte2tsx/svelte-shims-v4.d.ts @@ -253,16 +253,3 @@ 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 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 43291dbae..bb934f658 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';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 + { 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 diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/binding/expected-svelte5.js b/packages/svelte2tsx/test/htmlx2jsx/samples/binding/expected-svelte5.js index 355227003..6c881fec9 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';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 + { 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 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 31e8b2747..7bb207fea 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';obj = __sveltets_binding_value(Input, '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';} \ No newline at end of file