From fba28b2ddf5da7834ba380c605fdf8a37e658c5d Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sat, 16 Nov 2024 19:00:17 +0100 Subject: [PATCH] fix: detect shadowed variables/types during type hoisting (#2590) #2589 --- .../src/svelte2tsx/nodes/Generics.ts | 6 ++ .../svelte2tsx/nodes/HoistableInterfaces.ts | 68 ++++++++++++++++--- .../processInstanceScriptContent.ts | 7 +- .../expectedv2.ts | 34 ++++++++++ .../input.svelte | 8 +++ .../expectedv2.ts | 15 ++++ .../input.svelte | 8 +++ .../expectedv2.ts | 15 ++++ .../input.svelte | 8 +++ 9 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/expectedv2.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/input.svelte create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/expectedv2.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/input.svelte create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/expectedv2.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/input.svelte diff --git a/packages/svelte2tsx/src/svelte2tsx/nodes/Generics.ts b/packages/svelte2tsx/src/svelte2tsx/nodes/Generics.ts index 79d1918a4..982626652 100644 --- a/packages/svelte2tsx/src/svelte2tsx/nodes/Generics.ts +++ b/packages/svelte2tsx/src/svelte2tsx/nodes/Generics.ts @@ -5,8 +5,10 @@ import { surroundWithIgnoreComments } from '../../utils/ignore'; import { throwError } from '../utils/error'; export class Generics { + /** The whole `T extends boolean` */ private definitions: string[] = []; private typeReferences: string[] = []; + /** The `T` in `T extends boolean` */ private references: string[] = []; genericsAttr: Node | undefined; @@ -93,6 +95,10 @@ export class Generics { return this.typeReferences; } + getReferences() { + return this.references; + } + toDefinitionString(addIgnore = false) { const surround = addIgnore ? surroundWithIgnoreComments : (str: string) => str; return this.definitions.length ? surround(`<${this.definitions.join(',')}>`) : ''; diff --git a/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts b/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts index 4337c3503..0ad8d535b 100644 --- a/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts +++ b/packages/svelte2tsx/src/svelte2tsx/nodes/HoistableInterfaces.ts @@ -167,11 +167,16 @@ export class HoistableInterfaces { } }); - this.interface_map.set(interface_name, { - type_deps: type_dependencies, - value_deps: value_dependencies, - node - }); + if (this.import_type_set.has(interface_name)) { + // shadowed; delete because we can't hoist + this.import_type_set.delete(interface_name); + } else { + this.interface_map.set(interface_name, { + type_deps: type_dependencies, + value_deps: value_dependencies, + node + }); + } } // Handle Type Alias Declarations @@ -188,12 +193,46 @@ export class HoistableInterfaces { generics ); - this.interface_map.set(alias_name, { - type_deps: type_dependencies, - value_deps: value_dependencies, - node + if (this.import_type_set.has(alias_name)) { + // shadowed; delete because we can't hoist + this.import_type_set.delete(alias_name); + } else { + this.interface_map.set(alias_name, { + type_deps: type_dependencies, + value_deps: value_dependencies, + node + }); + } + } + + // Handle top-level declarations: They could shadow module declarations; delete them from the set of allowed import values + if (ts.isVariableStatement(node)) { + node.declarationList.declarations.forEach((declaration) => { + if (ts.isIdentifier(declaration.name)) { + this.import_value_set.delete(declaration.name.text); + } }); } + + if (ts.isFunctionDeclaration(node) && node.name) { + this.import_value_set.delete(node.name.text); + } + + if (ts.isClassDeclaration(node) && node.name) { + this.import_value_set.delete(node.name.text); + } + + if (ts.isEnumDeclaration(node)) { + this.import_value_set.delete(node.name.text); + } + + if (ts.isTypeAliasDeclaration(node)) { + this.import_type_set.delete(node.name.text); + } + + if (ts.isInterfaceDeclaration(node)) { + this.import_type_set.delete(node.name.text); + } } analyze$propsRune( @@ -280,9 +319,18 @@ export class HoistableInterfaces { /** * Moves all interfaces that can be hoisted to the top of the script, if the $props rune's type is hoistable. */ - moveHoistableInterfaces(str: MagicString, astOffset: number, scriptStart: number) { + moveHoistableInterfaces( + str: MagicString, + astOffset: number, + scriptStart: number, + generics: string[] + ) { if (!this.props_interface.name) return; + for (const generic of generics) { + this.import_type_set.delete(generic); + } + const hoistable = this.determineHoistableInterfaces(); if (hoistable.has(this.props_interface.name)) { for (const [, node] of hoistable) { diff --git a/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts b/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts index e7d66870c..5b504ec6a 100644 --- a/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts +++ b/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts @@ -318,7 +318,12 @@ export function processInstanceScriptContent( transformInterfacesToTypes(tsAst, str, astOffset, nodesToMove); } - exportedNames.hoistableInterfaces.moveHoistableInterfaces(str, astOffset, script.start); + exportedNames.hoistableInterfaces.moveHoistableInterfaces( + str, + astOffset, + script.start, + generics.getReferences() + ); return { exportedNames, diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/expectedv2.ts new file mode 100644 index 000000000..bec5d85d2 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/expectedv2.ts @@ -0,0 +1,34 @@ +/// +; + type SomeType = T; + type T = unknown; +;;function render() { +;type $$ComponentProps = { someProp: SomeType; }; + let { someProp }:/*Ωignore_startΩ*/$$ComponentProps/*Ωignore_endΩ*/ = $props(); +; +async () => { + +}; +return { props: {} as any as $$ComponentProps, exports: {}, bindings: __sveltets_$$bindings(''), slots: {}, events: {} }} +class __sveltets_Render { + props() { + return render().props; + } + events() { + return render().events; + } + slots() { + return render().slots; + } + bindings() { return __sveltets_$$bindings(''); } + exports() { return {}; } +} + +interface $$IsomorphicComponent { + new (options: import('svelte').ComponentConstructorOptions['props']>>): import('svelte').SvelteComponent['props']>, ReturnType<__sveltets_Render['events']>, ReturnType<__sveltets_Render['slots']>> & { $$bindings?: ReturnType<__sveltets_Render['bindings']> } & ReturnType<__sveltets_Render['exports']>; + (internal: unknown, props: ReturnType<__sveltets_Render['props']> & {}): ReturnType<__sveltets_Render['exports']>; + z_$$bindings?: ReturnType<__sveltets_Render['bindings']>; +} +const Input__SvelteComponent_: $$IsomorphicComponent = null as any; +/*Ωignore_startΩ*/type Input__SvelteComponent_ = InstanceType>; +/*Ωignore_endΩ*/export default Input__SvelteComponent_; \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/input.svelte new file mode 100644 index 000000000..af13570b3 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/input.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/expectedv2.ts new file mode 100644 index 000000000..d7fe39298 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/expectedv2.ts @@ -0,0 +1,15 @@ +/// +; + let a = ''; +;;function render() { + + let a = true;;type $$ComponentProps = { someProp: typeof a }; + let { someProp }:/*Ωignore_startΩ*/$$ComponentProps/*Ωignore_endΩ*/ = $props(); +; +async () => { + +}; +return { props: {} as any as $$ComponentProps, exports: {}, bindings: __sveltets_$$bindings(''), slots: {}, events: {} }} +const Input__SvelteComponent_ = __sveltets_2_fn_component(render()); +type Input__SvelteComponent_ = ReturnType; +export default Input__SvelteComponent_; \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/input.svelte new file mode 100644 index 000000000..a892a8c9f --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/input.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/expectedv2.ts new file mode 100644 index 000000000..9455873cb --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/expectedv2.ts @@ -0,0 +1,15 @@ +/// +; + type Shadowed = string; +;;function render() { + + type Shadowed = boolean;;type $$ComponentProps = { someProp: Shadowed }; + let { someProp }:/*Ωignore_startΩ*/$$ComponentProps/*Ωignore_endΩ*/ = $props(); +; +async () => { + +}; +return { props: {} as any as $$ComponentProps, exports: {}, bindings: __sveltets_$$bindings(''), slots: {}, events: {} }} +const Input__SvelteComponent_ = __sveltets_2_fn_component(render()); +type Input__SvelteComponent_ = ReturnType; +export default Input__SvelteComponent_; \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/input.svelte new file mode 100644 index 000000000..3b2b98957 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/input.svelte @@ -0,0 +1,8 @@ + + +