sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.66k stars 4.13k forks source link

Svelte 5 migration script: transform slot usages #13419

Open dummdidumm opened 1 week ago

dummdidumm commented 1 week ago

Describe the problem

Right now the migration function leaves slot usages intact, because the component may not be under the user's control. On the other hand, it transforms slot creations into @render tags. This can create the odd situation where components within your project are transformed by the migration script but not its usage locations.

Describe the proposed solution

Add logic to migrate slot usages in components, e.g. <Component><div slot="foo">..</div></Component> turns into <Component>{#snippet foo()}<div>..</div>{/snippet}</Component> etc.

The tricky part is to figure out when to apply this. I see three options:

Option 3 is my current favorite.

Importance

nice to have

adiguba commented 1 week ago

For me, one of the problems with migration is that there is no intermediate mode.

=> A Svelte 4 component that use <slot/> will only accept slot. => A Svelte 5 component that use {@render} will only accept snippet.

Example :

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACs1TXUvDMBT9K5dMUGGsCPaldvXzH4hP1oeuud2ibRKSTByl_918jJlhwQkOfGvOzT3n3JPbnjSsRU2y557wqkOSkVspyZSYjXQH_Y6tQXvWYq1qh-S6VkyaouSlYZ0UysC96OSjv3gJjRIdnM6SCJsFktOr0ZZ0pCX9oeWO6bEuC89etWvKk53LPHLiTeeUvYNuhZmXxDDTYkmgRZPRylQFPGnGl748BbNCcChYuXxR9O57yJNFkSeWw3En--TftPqJ5kxKNOCVzgLF-eBqwclOMdw7QNSyJtvbw4iFlUr2naTHnDod0zpg6r8bOi3sdnaCsoYhJZlRaxymu2WOwjl0qW0qwcscSvLgPvyqOYu1ZRMcuSnJ_pLZg8baMMF3YRfxX7HW6NPN4Gug3AHgfEZPEib3L5hEjJbebFr0fVsY-hDnQiiKKoML-QFatIzCpK7rq1DsKrVk3Bax2yKyotRGH0OLqn5bKrHmNINJ0zQe9ikH0cPyTX-Tbx82AwYb8olUQuqzcyd7lPhTH_-NQu6DilZqiwUz17NoUf9z-i_DJ30y9Ly2BQAA

Therefore, if we migrate a component we must necessarily correct all the calling codes. Unfortunately this is not always possible, and it could generate a "status-quo" and prevent people from migrating.

Like for events, I think we need a "compatibility mode". A Svelte 5 component (using new syntax) should be able to convert the caller's slots into snippets.

Example here : (CompSvelte5 is just the same generated code than above, with a call to a convert_slots_to_snippets() function)

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACo1UXW_TQBD8K4uxVBsFWyAhodR2QcArRap4qqvWsdfNtc7d6W6dUCL_d-7DNmlKAeXF3t2bmZvZeB-0rEMdLC_3Aa82GCyDj1IGi4AepH3RW-wIzbsWvaptJdO1YpKKkpfENlIogk9iIy_c4DtoldjASZIe1JI7fXJa8iydT2YHXQeUNWwLuhOUlwEx6rAMoENaNhVVBXzXjN-69gJojWCrwDRkq2Jvn4csXRVZajAsdvoY_AnX_qXmTEokcEyRh4gH2_NKipHQj_0HpwFNx-nhqQLj3kY0rGXYBEtSPQ6L2eyDyd-m3-lDw0eTX0GlIfT-lmMuKeOEilddWncMOZWB8Xmc30Mt-BYVXVvj9DWJ61GihmGOyfVsQOYSfFspUEIQ5BAmhBvZVYTRTaaxJiY41F2ldT6Rv35z37X3P9-XQeFMO9yCXiN8UMgbVLBjtH5eSxTD0tsI2Qtj6MhV3MSnVhL-cJdpsK36jqDtuZdyQBaFYcXrtVALCEOphNQx7G0mz3NOc5aCwkT2ej3VTNomIU9emvUgn3xuPP9sH5xxdiFqo0DwyfSSrHmTUbmzMYrnBhcNOlc1W3Vmt6Iwqdesa6LxRLyAtxOnGfI6I3tqAcahvJiulridhbMzg8WFkFM72nudQzxdSqE2EBP-WKwMLm8OHDvqSyGd6uFvO-v8fG5b58jmqP4dg4-LtTBVkjB005DnOfRmi1rGsRnnSlJIveJO8uA9a4WCyBBpAisSRAvnqztzt-QW6XzHvxlUo-Hhq2nqY5Z4BrYS3HnGj3bJ_mitxA447uCLUkJFpfn38rZjBCukHSJ3nyioeAP2aBl4V71Icvto9LV2PY4UXFrSq3F67PmamY2mrMxVKnXr4p41tfyoHcWPWIc_Znk1_AICGFty-AUAAA==

paoloricciuti commented 1 week ago

It's much simpler to do the third option @dummdidumm proposed and allow for compatibility between slots and snippets

adiguba commented 1 week ago

I think that a "compatibility layer" is an important thing to have (for slots, but also for on: directive), but I don't think it's a good idea to do this for all components.

=> The problem mainly concerns library's components, and the solution should be limited to that, without impacting all other components.

I know the idea of ​​adding a svelte:option for this is not popular. But I ask myself if we could handle this via the props() rune...

Something like :

<script>
    let {
         header, footer,
         onclick, onblur, onfocus,
    } = $props({
        // Declare slots that will be mapped to snippets
        slots: ['header', 'footer'],
        // Declare the on:event directive that will be mapped to handlers props
        events: ['click', 'blur', 'focus']
    });
</script>
paoloricciuti commented 1 week ago

If there has to be a compatibility layer I don't see why having general compatibility is bad considering the two are already pretty close and Simon already did the work to make them compatible.

The on directive is already solved by the bubble function

paoloricciuti commented 1 week ago

=> The problem mainly concerns library's components, and the solution should be limited to that, without impacting all other components.

And the nice thing is that this will not impact other components since the code is already basically the same you can take a look at the pr

adiguba commented 1 week ago

I think a general compatibility could allow people to use slots on Svelte5 components designed with snippets.

I would prefer to have a way to handle this on a case-by-case basis.

And for the bubble, this brings its disadvantage in Svelte 5 => adding an handler even if the caller not use it.

paoloricciuti commented 1 week ago

I think a general compatibility could allow people to use slots on Svelte5 components designed with snippets.

I don't get why it's a bad thing...if you have a component that you can't migrate you are stuck to write svelte 4 components it you need to use them in that component. With interop you can write new components in 5 and use them in difficult to migrate components.

I would prefer to have a way to handle this on a case-by-case basis.

And for the bubble, this brings its disadvantage in Svelte 5 => adding an handler even if the caller not use it.

Yeah but you can't change that because you don't know if the component you are attaching too is 4 or 5

adiguba commented 1 week ago

Some reasons :

This introduces magic, without a way to control it.

=> I am convinced that there must be a way to migrate components in a compatible way, but I think that this should not be a global system, but a controllable case-by-case basis.

adiguba commented 1 week ago

I really think there should be a way to control this, and to transform slots and on:event into snippet/props ONLY for the components that we want to migrate while managing compatibility.

Imagine a Svelte component like this :

<script>

    export let title = "..."
    let copyright = "...";
    let year = 2024;

</script>

<slot name="header" />

<slot name="footer" {year} {copyright} />

<slot name="title" {title}>
    <b>{title}</b>
</slot>

Using the migration tool, we currently have something like this :

<script>

    let {
        title = "...",
        header,
        footer,
        title_1
    } = $props();

    let copyright = "...";
    let year = 2024;

</script>

{@render header?.()}

{@render footer?.({ year, copyright, })}

{#if title_1}
    {@render title_1({ title, })}
{:else}
    <b>{title}</b>
{/if}

But, for the new Svelte 5 component, I may want :

Perhaps it could become something like this :

<script>
    // $legacy will describe how received slots will be handled,
    // to ensure compatibility with older codes
    $legacy({
        slots: {
            // header was a simple slot without variable
            // this allows it to be mapped to the prop snippet of the same name
            header: true,

            // footer was a slot with a variable
            // this allows it to be mapped to the prop snippet of the same name
            // and convert the snippet arguments into an object for the slot
            footer: (copyright, year) => ({copyright, year}),

            // title was a slot with a conflicting name and a variable
            title: {
                // it will be converted to the snippet "titleTemplate"
                snippet: 'titleTemplate',
                // using this mapping function to convert the snippet arguments :
                map: (title) => ({title})
            }
        }
    })

    let {
        title = "...",
        header,
        titleTemplate,
        footer,
    } = $props();

    let copyright = "...";
    let year = 2024;

</script>

<!-- <slot name="header" /> -->
{@render header?.()}

<!-- <slot name="footer" {year} {copyright} /> -->
{@render footer?.(copyright, year)}

{#if titleTemplate}
    <!-- <slot name="title" {title} /> -->
    {@render titleTemplate(title)}
{:else}
    <b>{title}</b>
{/if}

So the component could be used using snippet and the new syntax :

<Comp title="the title">
    {#snippet header}HEADER{/snippet}
    {#snippet titleTemplate(title)}{title}{/snippet}
    {#snippet footer(copyright, year)} {copyright} - {year} {/snippet}
</Comp>

Or with the old-syntax with the slot for codes that haven't be migrated :

<Comp title="the title">
    <div slot="header">HEADER</div>
    <div slot="title" let:title>{title}</div>
    <div slot="header" let:copyright let:year> {copyright} - {year}</div>
</Comp>
adiguba commented 1 week ago

We can even use it on new component to completely ban the use of slots :


    $legacy({
        // throw an Error if the component receive a slot
        slots: false
    });
paoloricciuti commented 1 week ago

You keep coming up with new APIs that only matter in the transition phase. That's bad because you can't get rid of them during the lifecycle of svelte 5.

The change suggested by Simon is ok because it's not impactful at the API level. It's unreasonable to think that the migration script should produce a component just like the user want it and it should be just a jumpstart for you to migrate to use rune if you don't have the capacity to migrate that component by hand.

adiguba commented 1 week ago

Why you can't get rid of it ? When you deprecate slot/on:directive you can also deprecate this runes...

And I don't say that the migration tools must do that. But the developer who maintaining the library should have a way to handle this.