sveltejs / svelte

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

Svelte5 : melt-ui preprocessor caused `{@const}` values to be used before initialization. #11450

Open sparecycles opened 2 weeks ago

sparecycles commented 2 weeks ago

Describe the bug

Filed an issue in melt-ui, https://github.com/melt-ui/preprocessor/issues/57 Cross-posting for visibility in case this actually is a larger issue in svelte's code gen emit ordering.

Reproduction

Starting with melt-ui's basic stackblitz setup:

See {@const itemId = id} usage in here: https://stackblitz.com/edit/github-suxd85?file=src%2Flib%2Fcomponents%2FAccordion.svelte

In case the stackblitz repo is broken, run

npm i svelte@next

and change

 <div class="mx-auto w-full max-w-md rounded-md shadow-lg" {...$root}>
    {#each items as { id, title, description }, i}
+       {@const itemId = id}
        <div
-           use:melt={$item(id)}
+           use:melt={$item(itemId)}

Logs

Uncaught ReferenceError: can't access lexical declaration 'itemId' before initialization
    __MELTUI_BUILDER_0__ Accordion.svelte:30
    execute_reaction_fn chunk-OJQLA3TO.js:1070
    update_derived chunk-OJQLA3TO.js:847
    get chunk-OJQLA3TO.js:1405
    Accordion Accordion.svelte:98

Severity

annoyance

AdrianGonz97 commented 2 weeks ago

This issue can be boiled down to if the order of @const tag declarations now matter in Svelte 5 and if it's an intended breaking change:

Svelte 4

Svelte 5

dummdidumm commented 2 weeks ago

I think we should preserve the Svelte 4 behavior in legacy mode but introduce the more strict behavior in runes mode (so that we can eventually remove the logic of reordering them)

sparecycles commented 2 weeks ago

Svelte4 had too much magic. 😁

There's a third option which is requiring an additional flag for this logic (if it's primarily an issue for library-generated code, since client code can always reorder easily):

Pro

Con