melt-ui / preprocessor

19 stars 4 forks source link

[Svelte5] Cannot access {@const} before initialization #57

Closed sparecycles closed 3 months ago

sparecycles commented 3 months ago

Describe the bug

@const-declared values get hoisted above their definitions in latest svelte 5 (svelte@5.0.0-next.123 as of today).

A workaround might be to not use melt-pp for builder expressions that involve const values.

Reproduction

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 work, 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
    e chunk-ISEWF24H.js:822
    execute_reaction_fn chunk-OJQLA3TO.js:1070
    execute_effect chunk-OJQLA3TO.js:1187
    create_effect chunk-OJQLA3TO.js:368
    branch chunk-OJQLA3TO.js:449
    create_item chunk-ISEWF24H.js:822
    reconcile chunk-ISEWF24H.js:679
    each chunk-ISEWF24H.js:623
    execute_reaction_fn chunk-OJQLA3TO.js:1070

Generated code in svelte:

        const __MELTUI_BUILDER_0__ = $.derived_safe_equal(() => $item()($.get(itemId)));

        $.get(__MELTUI_BUILDER_0__);

        const itemId = $.derived_safe_equal(id);

System Info

.

Severity

annoyance

Workaround

Use moar {@const}, lol.

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

This is more so a flaw in the PP's hoisting logic than it is a Svelte 5 specific issue. However, it is interesting how it was working in Svelte 4 with no issues.

Here's the generated PP code for this situation:

<div class="mx-auto w-full max-w-md rounded-md shadow-lg" {...$root}>
    {#each items as { id, title, description }, i}
        {@const __MELTUI_BUILDER_0__ = $item(itemId)}
        {@const itemId = id}
        <div
            {...__MELTUI_BUILDER_0__} use:__MELTUI_BUILDER_0__ .action
            class="overflow-hidden transition-colors first:rounded-t
            last:rounded-b focus-within:relative focus-within:z-10 focus-within:ring
            focus-within:ring-magnum-400"
        >

<!-- ... -->

The PP doesn't take into consideration existing @const declarations. Will look to fix this soon.


Out of curiosity though, what's the usecase for the current configuration of just redeclaring the id? Why use an @const declaration instead of just renaming the value in the destructure? (e.g. {#each items as { id: itemId, title, description }, i})

sparecycles commented 3 months ago

In my project, I was using hasChildren as a short-hand in a melt-ui TreeView like this,

  {#each tree as { title, id: itemId, icon, children }, i}
    {@const hasChildren = !!children?.length}
    ...
      <button
        ...
        use:melt={$item({
          id: itemId,
          hasChildren,
        })}
      >

I just recreated the problem in an even less compelling form to demonstrate the issue. 😉

sparecycles commented 3 months ago

Based on the existing PP output, it looks like either svelte4 might either be solving const problems too hard or has its own bug that's cancelling out the melt preprocessor one! 🤔

Anyway, thanks for taking a look.

AdrianGonz97 commented 3 months ago

This is now fixed as of version 0.3.2!