fix: implict children tweaks (#2285)

- ignore comments when determining whether or not to add an implicit children prop #2263 - do static analysis of the destructuring to see whether or not children is destructured from `$props()`, and if not, add the implicit children prop if there's a slot. This results in less confusing type errors when transforming to runes mode and using a slot
sveltejs · Feb 7, 2024 · efe8463 · efe8463
1 parent 19694d6
commit efe8463
Show file tree

Hide file tree

Showing 16 changed files with 202 additions and 69 deletions.
diff --git a/packages/svelte2tsx/src/svelte2tsx/createRenderFunction.ts b/packages/svelte2tsx/src/svelte2tsx/createRenderFunction.ts
@@ -111,7 +111,7 @@ export function createRenderFunction({
 
     const needsImplicitChildrenProp =
         svelte5Plus &&
-        !exportedNames.uses$propsRune() &&
+        !exportedNames.usesChildrenIn$propsRune() &&
         slots.has('default') &&
         !exportedNames.getExportsMap().has('default');
     if (needsImplicitChildrenProp) {

diff --git a/packages/svelte2tsx/src/svelte2tsx/nodes/ExportedNames.ts b/packages/svelte2tsx/src/svelte2tsx/nodes/ExportedNames.ts
@@ -17,7 +17,8 @@ interface ExportedName {
     identifierText?: string;
     required?: boolean;
     doc?: string;
-    implicitChildren?: 'empty' | 'attributes';
+    /** Set if this is the implicit children prop. `empty` == no parameters, else `has_params` */
+    implicitChildren?: 'empty' | 'has_params';
 }
 
 export class ExportedNames {
@@ -31,7 +32,8 @@ export class ExportedNames {
      */
     private $props = {
         comment: '',
-        generic: ''
+        generic: '',
+        mayHaveChildrenProp: false
     };
     private exports = new Map<string, ExportedName>();
     private possibleExports = new Map<
@@ -135,6 +137,29 @@ export class ExportedNames {
             initializer: ts.CallExpression & { expression: ts.Identifier };
         }
     ): void {
+        // Check if the $props() rune has a children prop
+        if (ts.isObjectBindingPattern(node.name)) {
+            for (const element of node.name.elements) {
+                if (
+                    !ts.isIdentifier(element.name) ||
+                    (element.propertyName && !ts.isIdentifier(element.propertyName)) ||
+                    !!element.dotDotDotToken
+                ) {
+                    // not statically analyzable, so we assume it may have children
+                    this.$props.mayHaveChildrenProp = true;
+                } else {
+                    const name = element.propertyName
+                        ? (element.propertyName as ts.Identifier).text
+                        : element.name.text;
+                    if (name === 'children') {
+                        this.$props.mayHaveChildrenProp = true;
+                    }
+                }
+            }
+        } else {
+            this.$props.mayHaveChildrenProp = true;
+        }
+
         if (node.initializer.typeArguments?.length > 0) {
             const generic_arg = node.initializer.typeArguments[0];
             const generic = generic_arg.getText();
@@ -201,7 +226,8 @@ export class ExportedNames {
             }
 
             if (internalHelpers.isKitRouteFile(this.basename)) {
-                const kitType = this.basename.includes('layout')
+                this.$props.mayHaveChildrenProp = this.basename.includes('layout');
+                const kitType = this.$props.mayHaveChildrenProp
                     ? `{ data: import('./$types.js').LayoutData, form: import('./$types.js').ActionData, children: import('svelte').Snippet }`
                     : `{ data: import('./$types.js').PageData, form: import('./$types.js').ActionData }`;
 
@@ -498,12 +524,12 @@ export class ExportedNames {
         }
     }
 
-    addImplicitChildrenExport(hasAttributes: boolean): void {
+    addImplicitChildrenExport(hasParams: boolean): void {
         if (this.exports.has('children')) return;
 
         this.exports.set('children', {
             isLet: true,
-            implicitChildren: hasAttributes ? 'attributes' : 'empty'
+            implicitChildren: hasParams ? 'has_params' : 'empty'
         });
     }
 
@@ -589,7 +615,9 @@ export class ExportedNames {
         const names = Array.from(this.exports.entries());
 
         if (this.$props.generic) {
-            const others = names.filter(([, { isLet }]) => !isLet);
+            const others = names.filter(
+                ([, { isLet, implicitChildren }]) => !isLet || !!implicitChildren
+            );
             return (
                 '{} as any as ' +
                 this.$props.generic +
@@ -600,8 +628,23 @@ export class ExportedNames {
         }
 
         if (this.$props.comment) {
-            // TODO: createReturnElements would need to be incorporated here, but don't bother for now.
-            // In the long run it's probably better to have them on a different object anyway.
+            // Try our best to incorporate createReturnElementsType here
+            const others = names.filter(
+                ([, { isLet, implicitChildren }]) => !isLet || !!implicitChildren
+            );
+            let idx = this.$props.comment.indexOf('@type');
+            if (idx !== -1 && /[\s{]/.test(this.$props.comment[idx + 5]) && others.length > 0) {
+                idx = this.$props.comment.indexOf('{', idx);
+                if (idx !== -1) {
+                    idx++;
+                    return (
+                        this.$props.comment.slice(0, idx) +
+                        `{${this.createReturnElementsType(others, false)}} & ` +
+                        this.$props.comment.slice(idx) +
+                        '({})'
+                    );
+                }
+            }
             return this.$props.comment + '({})';
         }
 
@@ -666,7 +709,7 @@ export class ExportedNames {
         });
     }
 
-    private createReturnElementsType(names: Array<[string, ExportedName]>) {
+    private createReturnElementsType(names: Array<[string, ExportedName]>, addDoc = true) {
         return names.map(([key, value]) => {
             if (value.implicitChildren) {
                 return `children?: ${
@@ -676,9 +719,9 @@ export class ExportedNames {
                 }`;
             }
 
-            const identifier = `${value.doc ? `\n${value.doc}` : ''}${value.identifierText || key}${
-                value.required ? '' : '?'
-            }`;
+            const identifier = `${value.doc && addDoc ? `\n${value.doc}` : ''}${
+                value.identifierText || key
+            }${value.required ? '' : '?'}`;
             if (!value.type) {
                 return `${identifier}: typeof ${key}`;
             }
@@ -697,7 +740,7 @@ export class ExportedNames {
         return this.exports;
     }
 
-    uses$propsRune() {
-        return !!this.$props.generic || !!this.$props.comment;
+    usesChildrenIn$propsRune() {
+        return this.$props.mayHaveChildrenProp;
     }
 }
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expected-svelte5.ts b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expected-svelte5.ts
@@ -0,0 +1,17 @@
+///<reference types="svelte" />
+;function render() {
+
+    /** @type {SomeType} */
+    let { a, b } = $props();
+    let x = $state(0);
+    let y = $derived(x * 2);
+
+/*Ωignore_startΩ*/;const __sveltets_createSlot = __sveltets_2_createCreateSlot();/*Ωignore_endΩ*/;
+async () => {
+
+ { __sveltets_createSlot("default", {  x,y,});}};
+let $$implicit_children = __sveltets_2_snippet({x:x, y:y});
+return { props: /** @type {{children?: typeof $$implicit_children} & SomeType} */({}), slots: {'default': {x:x, y:y}}, events: {} }}
+
+export default class Input__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_partial(['children'], __sveltets_2_with_any_event(render()))) {
+}
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expectedv2.ts
@@ -0,0 +1,16 @@
+///<reference types="svelte" />
+;function render() {
+
+    /** @type {SomeType} */
+    let { a, b } = $props();
+    let x = $state(0);
+    let y = $derived(x * 2);
+
+/*Ωignore_startΩ*/;const __sveltets_createSlot = __sveltets_2_createCreateSlot();/*Ωignore_endΩ*/;
+async () => {
+
+ { __sveltets_createSlot("default", {  x,y,});}};
+return { props: /** @type {SomeType} */({}), slots: {'default': {x:x, y:y}}, events: {} }}
+
+export default class Input__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_partial(__sveltets_2_with_any_event(render()))) {
+}
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/input.svelte
@@ -0,0 +1,8 @@
+<script>
+    /** @type {SomeType} */
+    let { a, b } = $props();
+    let x = $state(0);
+    let y = $derived(x * 2);
+</script>
+
+<slot {x} {y} />
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/runes/expectedv2.ts
@@ -1,16 +1,13 @@
 ///<reference types="svelte" />
 ;function render() {
 
-    /** @type {a: number, b: string} */
+    /** @typedef {{a: number, b: string}}  $$_sveltets_Props *//** @type {$$_sveltets_Props} */
     let { a, b } = $props();
     let x = $state(0);
     let y = $derived(x * 2);
-
-/*Ωignore_startΩ*/;const __sveltets_createSlot = __sveltets_2_createCreateSlot();/*Ωignore_endΩ*/;
-async () => {
-
- { __sveltets_createSlot("default", {  x,y,});}};
-return { props: /** @type {a: number, b: string} */({}), slots: {'default': {x:x, y:y}}, events: {} }}
+;
+async () => {};
+return { props: /** @type {$$_sveltets_Props} */({}), slots: {}, events: {} }}
 
 export default class Input__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_partial(__sveltets_2_with_any_event(render()))) {
 }
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/runes/input.svelte
@@ -1,8 +1,6 @@
 <script>
-    /** @type {a: number, b: string} */
+    /** @type {{a: number, b: string}} */
     let { a, b } = $props();
     let x = $state(0);
     let y = $derived(x * 2);
 </script>
-
-<slot {x} {y} />
diff --git a/...elte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune-no-changes/expectedv2.ts b/...elte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune-no-changes/expectedv2.ts
@@ -7,7 +7,7 @@
      const snapshot = {};
 ;
 async () => {};
-return { props: /** @type {$$_sveltets_Props} */({}), slots: {}, events: {} }}
+return { props: /** @type {{snapshot?: typeof snapshot} & $$_sveltets_Props} */({}), slots: {}, events: {} }}
 
 export default class Page__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_partial(['snapshot'], __sveltets_2_with_any_event(render()))) {
     get snapshot() { return __sveltets_2_nonNullable(this.$$prop_def.snapshot) }

diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune/expectedv2.ts
@@ -5,7 +5,7 @@
      const snapshot/*Ωignore_startΩ*/: import('./$types.js').Snapshot/*Ωignore_endΩ*/ = {};
 ;
 async () => {};
-return { props: /** @type {{ data: import('./$types.js').PageData, form: import('./$types.js').ActionData }} */({}), slots: {}, events: {} }}
+return { props: /** @type {{snapshot?: typeof snapshot} & { data: import('./$types.js').PageData, form: import('./$types.js').ActionData }} */({}), slots: {}, events: {} }}
 
 export default class Page__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_partial(['snapshot'], __sveltets_2_with_any_event(render()))) {
     get snapshot() { return __sveltets_2_nonNullable(this.$$prop_def.snapshot) }

diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-generics/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-generics/expectedv2.ts
@@ -1,15 +1,25 @@
 ///<reference types="svelte" />
-;function render() {
-;type $$_sveltets_Props = { a: number, b: string };
+;function render<T>() {
+;type $$_sveltets_Props = { a: T, b: string };
     let { a, b } = $props<$$_sveltets_Props>();
-    let x = $state(0);
+    let x = $state<T>(0);
     let y = $derived(x * 2);
+;
+async () => {};
+return { props: {} as any as $$_sveltets_Props, slots: {}, events: {} }}
+class __sveltets_Render<T> {
+    props() {
+        return render<T>().props;
+    }
+    events() {
+        return __sveltets_2_with_any_event(render<T>()).events;
+    }
+    slots() {
+        return render<T>().slots;
+    }
+}
 
-/*Ωignore_startΩ*/;const __sveltets_createSlot = __sveltets_2_createCreateSlot();/*Ωignore_endΩ*/;
-async () => {
 
- { __sveltets_createSlot("default", {  x,y,});}};
-return { props: {} as any as $$_sveltets_Props, slots: {'default': {x:x, y:y}}, events: {} }}
-
-export default class Input__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_with_any_event(render())) {
+import { SvelteComponentTyped as __SvelteComponentTyped__ } from "svelte" 
+export default class Input__SvelteComponent_<T> extends __SvelteComponentTyped__<ReturnType<__sveltets_Render<T>['props']>, ReturnType<__sveltets_Render<T>['events']>, ReturnType<__sveltets_Render<T>['slots']>> {
 }
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-generics/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-generics/input.svelte
@@ -1,7 +1,5 @@
-<script lang="ts">
-    let { a, b } = $props<{ a: number, b: string }>();
-    let x = $state(0);
+<script lang="ts" generics="T">
+    let { a, b } = $props<{ a: T, b: string }>();
+    let x = $state<T>(0);
     let y = $derived(x * 2);
 </script>
-
-<slot {x} {y} />
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expected-svelte5.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expected-svelte5.ts
@@ -0,0 +1,29 @@
+///<reference types="svelte" />
+;function render<T>() {
+;type $$_sveltets_Props = { a: T, b: string };
+    let { a, b } = $props<$$_sveltets_Props>();
+    let x = $state<T>(0);
+    let y = $derived(x * 2);
+
+/*Ωignore_startΩ*/;const __sveltets_createSlot = __sveltets_2_createCreateSlot();/*Ωignore_endΩ*/;
+async () => {
+
+ { __sveltets_createSlot("default", {  x,y,});}};
+let $$implicit_children = __sveltets_2_snippet({x:x, y:y});
+return { props: {} as any as $$_sveltets_Props & { children?: typeof $$implicit_children }, slots: {'default': {x:x, y:y}}, events: {} }}
+class __sveltets_Render<T> {
+    props() {
+        return render<T>().props;
+    }
+    events() {
+        return __sveltets_2_with_any_event(render<T>()).events;
+    }
+    slots() {
+        return render<T>().slots;
+    }
+}
+
+
+import { SvelteComponentTyped as __SvelteComponentTyped__ } from "svelte" 
+export default class Input__SvelteComponent_<T> extends __SvelteComponentTyped__<ReturnType<__sveltets_Render<T>['props']>, ReturnType<__sveltets_Render<T>['events']>, ReturnType<__sveltets_Render<T>['slots']>> {
+}
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expectedv2.ts
@@ -0,0 +1,28 @@
+///<reference types="svelte" />
+;function render<T>() {
+;type $$_sveltets_Props = { a: T, b: string };
+    let { a, b } = $props<$$_sveltets_Props>();
+    let x = $state<T>(0);
+    let y = $derived(x * 2);
+
+/*Ωignore_startΩ*/;const __sveltets_createSlot = __sveltets_2_createCreateSlot();/*Ωignore_endΩ*/;
+async () => {
+
+ { __sveltets_createSlot("default", {  x,y,});}};
+return { props: {} as any as $$_sveltets_Props, slots: {'default': {x:x, y:y}}, events: {} }}
+class __sveltets_Render<T> {
+    props() {
+        return render<T>().props;
+    }
+    events() {
+        return __sveltets_2_with_any_event(render<T>()).events;
+    }
+    slots() {
+        return render<T>().slots;
+    }
+}
+
+
+import { SvelteComponentTyped as __SvelteComponentTyped__ } from "svelte" 
+export default class Input__SvelteComponent_<T> extends __SvelteComponentTyped__<ReturnType<__sveltets_Render<T>['props']>, ReturnType<__sveltets_Render<T>['events']>, ReturnType<__sveltets_Render<T>['slots']>> {
+}
diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/input.svelte
@@ -0,0 +1,7 @@
+<script lang="ts" generics="T">
+    let { a, b } = $props<{ a: T, b: string }>();
+    let x = $state<T>(0);
+    let y = $derived(x * 2);
+</script>
+
+<slot {x} {y} />