sveltejs / svelte

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

Bad compilation of valid Svelte 4 components under Svelte 5 #12254

Open arackaf opened 4 days ago

arackaf commented 4 days ago

Describe the bug

Repo is here https://github.com/arackaf/svelte-5-build-debug

Deploy it to Vercel, and you'll see these errors

image

If you check the git history, commit 1c2912b229c7a4efbb41f444e3ee7584f7d7ced5 imports adapter-vercel in order to set edge runtime settings. If you revert that commit, then the deployment suddenly works.

So it looks like adapter-vercel is doing something weird.

Reproduction

See above

Logs

n/a

System Info

System:
    OS: macOS 14.4.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 976.52 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - /usr/local/Cellar/nvm/0.38.0/versions/node/v20.10.0/bin/node
    Yarn: 1.22.22 - /usr/local/Cellar/nvm/0.38.0/versions/node/v20.10.0/bin/yarn
    npm: 10.2.3 - /usr/local/Cellar/nvm/0.38.0/versions/node/v20.10.0/bin/npm
    pnpm: 8.14.0 - /usr/local/Cellar/nvm/0.38.0/versions/node/v20.10.0/bin/pnpm
    bun: 1.0.36 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 126.1.67.119
    Chrome: 126.0.6478.127
  npmPackages:
    svelte: ^5.0.0-next.1 => 5.0.0-next.168

Severity

blocking an upgrade

Rich-Harris commented 4 days ago

No need to deploy; pnpm build will suffice.

The reason adapter-vercel is relevant is that it introduces esbuild into the mix, in order to generate self-contained JS modules for the edge runtime. esbuild fails if it sees an assignment to a constant.

Given code like this...

<script>
  import Input from "./Input.svelte";
</script>

{#each ['tom', 'dick', 'harry'] as author}
  <Input bind:value={author} />
{/each}

...Svelte generates this wall of gibberish in SSR mode:

import * as $ from "svelte/internal/server";
import Input from "./Input.svelte";

export default function App($$payload) {
  let $$settled = true;
  let $$inner_payload;

  function $$render_inner($$payload) {
    const each_array = $.ensure_array_like(['tom', 'dick', 'harry']);

    $$payload.out += `<!--[-->`;

    for (let $$index = 0; $$index < each_array.length; $$index++) {
      const author = each_array[$$index];

      $$payload.out += "<!--[-->";
      $$payload.out += `<!--[-->`;

      Input($$payload, {
        get value() {
          return author;
        },
        set value($$value) {
          author = $$value;
          $$settled = false;
        }
      });

      $$payload.out += `<!--]-->`;
      $$payload.out += "<!--]-->";
    }

    $$payload.out += "<!--]-->";
  };

  do {
    $$settled = true;
    $$inner_payload = $.copy_payload($$payload);
    $$render_inner($$inner_payload);
  } while (!$$settled);

  $.assign_payload($$payload, $$inner_payload);
}

This all exists because in Svelte 4, if a child component initializes a binding whose value is undefined, the change has to propagate back up to the parent. This is a rare edge case that significantly slows down SSR and only happens if you've built your app badly (bindings should always have an initial value) and as such we skip all that in runes mode.

(Side-note: if you opt the component into runes mode (which needs <svelte:options runes> in this case since it's not using any runes) then you get a compile error, because updating the current each block value is like reassigning a function parameter — it makes no sense. Svelte 4 tries to figure out which values are actually being updated when you do that, but it frequently gets it wrong and causes cascading invalidations, so this sort of reassignment is forbidden in runes mode.)

Luckily, this is an easy fix, because Svelte 4 doesn't properly invalidate everything — it just... reassigns the function parameter:

do {
  $$settled = true;

  // $$result.head is mutated by the literal expression
  // need to reset it if we're looping back to prevent duplication
  $$result.head = previous_head;

  $$rendered = `${each(authors, author => {
    return `${validate_component(Input, "Input").$$render(
      $$result,
      { value: author },
      {
        value: $$value => {
          author = $$value;
          $$settled = false;
        }
      },
      {}
    )}`;
  })}`;
} while (!$$settled);

As such I think all we need to do is turn the const to a let and esbuild will be just fine with the whole thing.

arackaf commented 4 days ago

@Rich-Harris Can you please clarify this?

This all exists because in Svelte 4, if a child component initializes a binding whose value is undefined, the change has to propagate back up to the parent. This is a rare edge case that significantly slows down SSR and only happens if you've built your app badly (bindings should always have an initial value)

Which bindings are undefined in my code? If you mean the book prop that's passed through multiple layers, I can initialize it to null at each and every component, but the error remains.

Rich-Harris commented 4 days ago

They're not. But as far as the compiler is concerned, they could be