From efe8463e8d853cb726e9d0502ac4f42ed394ebf5 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:02:56 +0100 Subject: [PATCH] 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 --- .../src/svelte2tsx/createRenderFunction.ts | 2 +- .../src/svelte2tsx/nodes/ExportedNames.ts | 71 +++++++++++++++---- .../runes-with-slots/expected-svelte5.ts | 17 +++++ .../samples/runes-with-slots/expectedv2.ts | 16 +++++ .../samples/runes-with-slots/input.svelte | 8 +++ .../svelte2tsx/samples/runes/expectedv2.ts | 11 ++- .../svelte2tsx/samples/runes/input.svelte | 4 +- .../expectedv2.ts | 2 +- .../expectedv2.ts | 2 +- .../samples/ts-runes-generics/expectedv2.ts | 28 +++++--- .../samples/ts-runes-generics/input.svelte | 8 +-- .../ts-runes-with-slot/expected-svelte5.ts | 29 ++++++++ .../samples/ts-runes-with-slot/expectedv2.ts | 28 ++++++++ .../samples/ts-runes-with-slot/input.svelte | 7 ++ .../svelte2tsx/samples/ts-runes/expectedv2.ts | 30 ++------ .../svelte2tsx/samples/ts-runes/input.svelte | 8 +-- 16 files changed, 202 insertions(+), 69 deletions(-) create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expected-svelte5.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expectedv2.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/input.svelte create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expected-svelte5.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expectedv2.ts create mode 100644 packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/input.svelte diff --git a/packages/svelte2tsx/src/svelte2tsx/createRenderFunction.ts b/packages/svelte2tsx/src/svelte2tsx/createRenderFunction.ts index c66c1a4e4..0f4e9fccb 100644 --- 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 index 3a8d8884b..4c1d28556 100644 --- 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(); 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 new file mode 100644 index 000000000..27aa1248f --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expected-svelte5.ts @@ -0,0 +1,17 @@ +/// +;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()))) { +} \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expectedv2.ts new file mode 100644 index 000000000..94529ba7f --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/expectedv2.ts @@ -0,0 +1,16 @@ +/// +;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()))) { +} \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/input.svelte new file mode 100644 index 000000000..5716aea3a --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/runes-with-slots/input.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/runes/expectedv2.ts index 7d5a49b02..e0ed9a5ca 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/runes/expectedv2.ts +++ b/packages/svelte2tsx/test/svelte2tsx/samples/runes/expectedv2.ts @@ -1,16 +1,13 @@ /// ;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()))) { } \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/runes/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/runes/input.svelte index 8053c403f..cbeb6fbe3 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/runes/input.svelte +++ b/packages/svelte2tsx/test/svelte2tsx/samples/runes/input.svelte @@ -1,8 +1,6 @@ - - diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune-no-changes/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune-no-changes/expectedv2.ts index 7701d6f03..2e4021ebb 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/sveltekit-autotypes-$props-rune-no-changes/expectedv2.ts +++ b/packages/svelte2tsx/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 index 4b82f6339..db3ad3c8b 100644 --- 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 index e3f235854..2710a577a 100644 --- 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 @@ /// -;function render() { -;type $$_sveltets_Props = { a: number, b: string }; +;function render() { +;type $$_sveltets_Props = { a: T, b: string }; let { a, b } = $props<$$_sveltets_Props>(); - let x = $state(0); + let x = $state(0); let y = $derived(x * 2); +; +async () => {}; +return { props: {} as any as $$_sveltets_Props, slots: {}, events: {} }} +class __sveltets_Render { + props() { + return render().props; + } + events() { + return __sveltets_2_with_any_event(render()).events; + } + slots() { + return render().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_ extends __SvelteComponentTyped__['props']>, ReturnType<__sveltets_Render['events']>, ReturnType<__sveltets_Render['slots']>> { } \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-generics/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-generics/input.svelte index f938f3025..bd52ebce3 100644 --- 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 @@ - - - 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 new file mode 100644 index 000000000..5c26bc094 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expected-svelte5.ts @@ -0,0 +1,29 @@ +/// +;function render() { +;type $$_sveltets_Props = { a: T, b: string }; + let { a, b } = $props<$$_sveltets_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: {} as any as $$_sveltets_Props & { children?: typeof $$implicit_children }, slots: {'default': {x:x, y:y}}, events: {} }} +class __sveltets_Render { + props() { + return render().props; + } + events() { + return __sveltets_2_with_any_event(render()).events; + } + slots() { + return render().slots; + } +} + + +import { SvelteComponentTyped as __SvelteComponentTyped__ } from "svelte" +export default class Input__SvelteComponent_ extends __SvelteComponentTyped__['props']>, ReturnType<__sveltets_Render['events']>, ReturnType<__sveltets_Render['slots']>> { +} \ No newline at end of file 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 new file mode 100644 index 000000000..c8b1ee4a1 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/expectedv2.ts @@ -0,0 +1,28 @@ +/// +;function render() { +;type $$_sveltets_Props = { a: T, b: string }; + let { a, b } = $props<$$_sveltets_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: {} as any as $$_sveltets_Props, slots: {'default': {x:x, y:y}}, events: {} }} +class __sveltets_Render { + props() { + return render().props; + } + events() { + return __sveltets_2_with_any_event(render()).events; + } + slots() { + return render().slots; + } +} + + +import { SvelteComponentTyped as __SvelteComponentTyped__ } from "svelte" +export default class Input__SvelteComponent_ extends __SvelteComponentTyped__['props']>, ReturnType<__sveltets_Render['events']>, ReturnType<__sveltets_Render['slots']>> { +} \ No newline at end of file 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 new file mode 100644 index 000000000..121a1d625 --- /dev/null +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes-with-slot/input.svelte @@ -0,0 +1,7 @@ + + + diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/expectedv2.ts b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/expectedv2.ts index c8b1ee4a1..633a6e8b2 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/expectedv2.ts +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/expectedv2.ts @@ -1,28 +1,12 @@ /// -;function render() { -;type $$_sveltets_Props = { a: T, b: string }; +;function render() { +;type $$_sveltets_Props = { a: number, b: string }; let { a, b } = $props<$$_sveltets_Props>(); - let x = $state(0); + let x = $state(0); let y = $derived(x * 2); +; +async () => {}; +return { props: {} as any as $$_sveltets_Props, slots: {}, events: {} }} -/*Ω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 { - props() { - return render().props; - } - events() { - return __sveltets_2_with_any_event(render()).events; - } - slots() { - return render().slots; - } -} - - -import { SvelteComponentTyped as __SvelteComponentTyped__ } from "svelte" -export default class Input__SvelteComponent_ extends __SvelteComponentTyped__['props']>, ReturnType<__sveltets_Render['events']>, ReturnType<__sveltets_Render['slots']>> { +export default class Input__SvelteComponent_ extends __sveltets_2_createSvelte2TsxComponent(__sveltets_2_with_any_event(render())) { } \ No newline at end of file diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/input.svelte b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/input.svelte index 121a1d625..dc191b092 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/input.svelte +++ b/packages/svelte2tsx/test/svelte2tsx/samples/ts-runes/input.svelte @@ -1,7 +1,5 @@ - - -