Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sparecycles opened this issue May 3, 2024 · 3 comments · Fixed by #11908
Closed
Assignees
Labels
Milestone

Comments

@sparecycles
Copy link

sparecycles commented May 3, 2024

Describe the bug

Filed an issue in melt-ui, melt-ui/preprocessor#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
Copy link

AdrianGonz97 commented May 3, 2024

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
Copy link
Member

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)

@dummdidumm dummdidumm added this to the 5.0 milestone May 3, 2024
@sparecycles
Copy link
Author

sparecycles commented May 3, 2024

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

  • encourages library developers to support the new rules (rather than tell users to apply the flag), and at the same time
  • unblocks the rest of the transition of code to runes for code reliant on broken libraries

Con

  • now users will have this flag in their code.
  • more compiler modes to test.
  • more to document.

@dummdidumm dummdidumm added the bug label May 16, 2024
@trueadm trueadm self-assigned this Jun 4, 2024
dummdidumm pushed a commit that referenced this issue Jun 6, 2024
Also remove create_block function in favor of calling visit which in turn calls the fragment visitor, to ensure scope is updated correctly
Fixes #11450
---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants