fix: detect shadowed variables/types during type hoisting (#2590)

#2589
sveltejs · Nov 16, 2024 · fba28b2 · fba28b2
1 parent bf2e459
commit fba28b2
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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/...ages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/expectedv2.ts b/...ages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-3.v5/expectedv2.ts
@@ -0,0 +1,34 @@
+///<reference types="svelte" />
+;
+    type SomeType<T extends boolean> = T;
+    type T = unknown;
+;;function render<T extends boolean>() {
+;type $$ComponentProps =  { someProp: SomeType<T>; };
+    let { someProp }:/*Ωignore_startΩ*/$$ComponentProps/*Ωignore_endΩ*/ = $props();
+;
+async () => {
+
+};
+return { props: {} as any as $$ComponentProps, exports: {}, bindings: __sveltets_$$bindings(''), slots: {}, events: {} }}
+class __sveltets_Render<T extends boolean> {
+    props() {
+        return render<T>().props;
+    }
+    events() {
+        return render<T>().events;
+    }
+    slots() {
+        return render<T>().slots;
+    }
+    bindings() { return __sveltets_$$bindings(''); }
+    exports() { return {}; }
+}
+
+interface $$IsomorphicComponent {
+    new <T extends boolean>(options: import('svelte').ComponentConstructorOptions<ReturnType<__sveltets_Render<T>['props']>>): import('svelte').SvelteComponent<ReturnType<__sveltets_Render<T>['props']>, ReturnType<__sveltets_Render<T>['events']>, ReturnType<__sveltets_Render<T>['slots']>> & { $$bindings?: ReturnType<__sveltets_Render<T>['bindings']> } & ReturnType<__sveltets_Render<T>['exports']>;
+    <T extends boolean>(internal: unknown, props: ReturnType<__sveltets_Render<T>['props']> & {}): ReturnType<__sveltets_Render<T>['exports']>;
+    z_$$bindings?: ReturnType<__sveltets_Render<any>['bindings']>;
+}
+const Input__SvelteComponent_: $$IsomorphicComponent = null as any;
+/*Ωignore_startΩ*/type Input__SvelteComponent_<T extends boolean> = InstanceType<typeof Input__SvelteComponent_<T>>;
+/*Ωignore_endΩ*/export default Input__SvelteComponent_;
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
@@ -0,0 +1,8 @@
+<script lang="ts" module>
+    type SomeType<T extends boolean> = T;
+    type T = unknown;
+</script>
+
+<script lang="ts" generics="T extends boolean">
+    let { someProp }: { someProp: SomeType<T>; } = $props();
+</script>
diff --git a/...ages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/expectedv2.ts b/...ages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-4.v5/expectedv2.ts
@@ -0,0 +1,15 @@
+///<reference types="svelte" />
+;
+    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<typeof Input__SvelteComponent_>;
+export default Input__SvelteComponent_;
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
@@ -0,0 +1,8 @@
+<script lang="ts" module>
+    let a = '';
+</script>
+
+<script lang="ts">
+    let a = true;
+    let { someProp }: { someProp: typeof a } = $props();
+</script>
diff --git a/...ages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/expectedv2.ts b/...ages/svelte2tsx/test/svelte2tsx/samples/ts-runes-hoistable-props-false-5.v5/expectedv2.ts
@@ -0,0 +1,15 @@
+///<reference types="svelte" />
+;
+    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<typeof Input__SvelteComponent_>;
+export default Input__SvelteComponent_;
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
@@ -0,0 +1,8 @@
+<script lang="ts" module>
+    type Shadowed = string;
+</script>
+
+<script lang="ts">
+    type Shadowed = boolean;
+    let { someProp }: { someProp: Shadowed } = $props();
+</script>