From 49a09081f0e65c73aec521455886b8b7de8a6a80 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Fri, 4 Oct 2024 10:12:17 +0200 Subject: [PATCH] fix: correctly migrate `$$slots` with bracket member expressions & slots with static props (#13468) - fix: correctly migrate `$$slots` with bracket member expressions - fix: wrong type for JSDoc `$$restProps` - fix: correctly migrate slots with static props - fix: `$$props` messing with `$$slots` Fixes #13467, fixes #13466, fixes #13472 --- .changeset/nine-kids-whisper.md | 5 ++ packages/svelte/src/compiler/migrate/index.js | 80 +++++++++++++++---- .../samples/props-rest-props/output.svelte | 2 +- .../samples/slots-custom-element/input.svelte | 4 + .../slots-custom-element/output.svelte | 8 +- .../samples/slots-with-$$props/input.svelte | 20 +++++ .../samples/slots-with-$$props/output.svelte | 27 +++++++ .../tests/migrate/samples/slots/input.svelte | 6 ++ .../tests/migrate/samples/slots/output.svelte | 8 +- .../samples/svelte-component/output.svelte | 2 +- 10 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 .changeset/nine-kids-whisper.md create mode 100644 packages/svelte/tests/migrate/samples/slots-with-$$props/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/slots-with-$$props/output.svelte diff --git a/.changeset/nine-kids-whisper.md b/.changeset/nine-kids-whisper.md new file mode 100644 index 000000000000..5b11743c520d --- /dev/null +++ b/.changeset/nine-kids-whisper.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly migrate `$$slots` with bracket member expressions & slots with static props diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index d60b2984f580..a033279e829e 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -192,7 +192,7 @@ export function migrate(source) { } } else { if (analysis.uses_props || analysis.uses_rest_props) { - type = `{Record}`; + type = `Record`; } else { type = `{${state.props .map((prop) => { @@ -294,7 +294,7 @@ export function migrate(source) { * str: MagicString; * analysis: ComponentAnalysis; * indent: string; - * props: Array<{ local: string; exported: string; init: string; bindable: boolean; slot_name?: string; optional: boolean; type: string; comment?: string, type_only?: boolean }>; + * props: Array<{ local: string; exported: string; init: string; bindable: boolean; slot_name?: string; optional: boolean; type: string; comment?: string; type_only?: boolean; needs_refine_type?: boolean; }>; * props_insertion_point: number; * has_props_rune: boolean; * end: number; @@ -704,7 +704,7 @@ const template = { handle_events(node, state); next(); }, - SlotElement(node, { state, next }) { + SlotElement(node, { state, next, visit }) { if (state.analysis.custom_element) return; let name = 'children'; let slot_name = 'default'; @@ -719,13 +719,22 @@ const template = { } else { const attr_value = attr.value === true || Array.isArray(attr.value) ? attr.value : [attr.value]; - const value = - attr_value !== true - ? state.str.original.substring( - attr_value[0].start, - attr_value[attr_value.length - 1].end - ) - : 'true'; + let value = 'true'; + if (attr_value !== true) { + const first = attr_value[0]; + const last = attr_value[attr_value.length - 1]; + for (const attr of attr_value) { + visit(attr); + } + value = state.str + .snip( + first.type === 'Text' + ? first.start - 1 + : /** @type {number} */ (first.expression.start), + last.type === 'Text' ? last.end + 1 : /** @type {number} */ (last.expression.end) + ) + .toString(); + } slot_props += value === attr.name ? `${value}, ` : `${attr.name}: ${value}, `; } } @@ -753,6 +762,9 @@ const template = { slot_name, type: `import('svelte').${slot_props ? 'Snippet<[any]>' : 'Snippet'}` }); + } else if (existing_prop.needs_refine_type) { + existing_prop.type = `import('svelte').${slot_props ? 'Snippet<[any]>' : 'Snippet'}`; + existing_prop.needs_refine_type = false; } if (node.fragment.nodes.length > 0) { @@ -760,11 +772,15 @@ const template = { state.str.update( node.start, node.fragment.nodes[0].start, - `{#if ${name}}{@render ${name}(${slot_props})}{:else}` + `{#if ${name}}{@render ${state.analysis.uses_props ? `${state.names.props}.` : ''}${name}(${slot_props})}{:else}` ); state.str.update(node.fragment.nodes[node.fragment.nodes.length - 1].end, node.end, '{/if}'); } else { - state.str.update(node.start, node.end, `{@render ${name}?.(${slot_props})}`); + state.str.update( + node.start, + node.end, + `{@render ${state.analysis.uses_props ? `${state.names.props}.` : ''}${name}?.(${slot_props})}` + ); } }, Comment(node, { state }) { @@ -958,7 +974,7 @@ function handle_identifier(node, state, path) { const parent = path.at(-1); if (parent?.type === 'MemberExpression' && parent.property === node) return; - if (state.analysis.uses_props) { + if (state.analysis.uses_props && node.name !== '$$slots') { if (node.name === '$$props' || node.name === '$$restProps') { // not 100% correct for $$restProps but it'll do state.str.update( @@ -980,10 +996,42 @@ function handle_identifier(node, state, path) { ); } else if (node.name === '$$slots' && state.analysis.uses_slots) { if (parent?.type === 'MemberExpression') { - state.str.update(/** @type {number} */ (node.start), parent.property.start, ''); - if (parent.property.name === 'default') { - state.str.update(parent.property.start, parent.property.end, 'children'); + if (state.analysis.custom_element) return; + + let name = parent.property.type === 'Literal' ? parent.property.value : parent.property.name; + let slot_name = name; + const existing_prop = state.props.find((prop) => prop.slot_name === name); + if (existing_prop) { + name = existing_prop.local; + } else if (name !== 'default') { + name = state.scope.generate(name); + } + + name = name === 'default' ? 'children' : name; + + if (!existing_prop) { + state.props.push({ + local: name, + exported: name, + init: '', + bindable: false, + optional: true, + slot_name, + // if it's the first time we encounter this slot + // we start with any and delegate to when the slot + // is actually rendered (it might not happen in that case) + // any is still a safe bet + type: `import('svelte').Snippet<[any]>}`, + needs_refine_type: true + }); } + + state.str.update( + /** @type {number} */ (node.start), + parent.property.start, + state.analysis.uses_props ? `${state.names.props}.` : '' + ); + state.str.update(parent.property.start, parent.end, name); } // else passed as identifier, we don't know what to do here, so let it error } else if ( diff --git a/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte b/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte index 7272cd5db683..fd6773a99939 100644 --- a/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte +++ b/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte @@ -1,5 +1,5 @@ diff --git a/packages/svelte/tests/migrate/samples/slots-custom-element/input.svelte b/packages/svelte/tests/migrate/samples/slots-custom-element/input.svelte index 3063b3251744..ee5d25a58e1b 100644 --- a/packages/svelte/tests/migrate/samples/slots-custom-element/input.svelte +++ b/packages/svelte/tests/migrate/samples/slots-custom-element/input.svelte @@ -20,4 +20,8 @@ {#if $$slots.default}foo{/if} +{#if $$slots['default']}foo{/if} + +{#if $$slots['dashed-name']}foo{/if} + diff --git a/packages/svelte/tests/migrate/samples/slots-custom-element/output.svelte b/packages/svelte/tests/migrate/samples/slots-custom-element/output.svelte index 8f530891a2dc..8c0d44a6ef68 100644 --- a/packages/svelte/tests/migrate/samples/slots-custom-element/output.svelte +++ b/packages/svelte/tests/migrate/samples/slots-custom-element/output.svelte @@ -13,11 +13,15 @@ {/if} -{#if bar} +{#if $$slots.bar} {$$slots} {/if} -{#if children}foo{/if} +{#if $$slots.default}foo{/if} + +{#if $$slots['default']}foo{/if} + +{#if $$slots['dashed-name']}foo{/if} \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/slots-with-$$props/input.svelte b/packages/svelte/tests/migrate/samples/slots-with-$$props/input.svelte new file mode 100644 index 000000000000..90812075c1bf --- /dev/null +++ b/packages/svelte/tests/migrate/samples/slots-with-$$props/input.svelte @@ -0,0 +1,20 @@ + + +{#if foo} + +{/if} + +{#if $$slots.bar} + {$$slots} + +{/if} + +{#if $$slots.default}foo{/if} + +{#if $$slots['default']}foo{/if} + +{#if $$slots['dashed-name']}foo{/if} + + + + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/slots-with-$$props/output.svelte b/packages/svelte/tests/migrate/samples/slots-with-$$props/output.svelte new file mode 100644 index 000000000000..b5cf1518e459 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/slots-with-$$props/output.svelte @@ -0,0 +1,27 @@ + + + + +{#if foo} + {@render props.foo_1?.({ foo, })} +{/if} + +{#if props.bar} + {$$slots} + {@render props.bar?.()} +{/if} + +{#if props.children}foo{/if} + +{#if props.children}foo{/if} + +{#if props.dashed_name}foo{/if} + +{@render props.dashed_name?.()} + +{@render props.children?.({ header: "something", title: props.cool, id, })} \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/slots/input.svelte b/packages/svelte/tests/migrate/samples/slots/input.svelte index d7bbede90631..3fea1b46f0cf 100644 --- a/packages/svelte/tests/migrate/samples/slots/input.svelte +++ b/packages/svelte/tests/migrate/samples/slots/input.svelte @@ -11,4 +11,10 @@ {#if $$slots.default}foo{/if} +{#if $$slots['default']}foo{/if} + +{#if $$slots['dashed-name']}foo{/if} + + + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/slots/output.svelte b/packages/svelte/tests/migrate/samples/slots/output.svelte index b04dbdea1831..9c56dbcf4305 100644 --- a/packages/svelte/tests/migrate/samples/slots/output.svelte +++ b/packages/svelte/tests/migrate/samples/slots/output.svelte @@ -21,4 +21,10 @@ {#if children}foo{/if} -{@render dashed_name?.()} \ No newline at end of file +{#if children}foo{/if} + +{#if dashed_name}foo{/if} + +{@render dashed_name?.()} + +{@render children?.({ header: "something", title: my_title, id, })} \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/svelte-component/output.svelte b/packages/svelte/tests/migrate/samples/svelte-component/output.svelte index 0fc5fff5557e..bba6fc6fa7a8 100644 --- a/packages/svelte/tests/migrate/samples/svelte-component/output.svelte +++ b/packages/svelte/tests/migrate/samples/svelte-component/output.svelte @@ -1,5 +1,5 @@