fix: ensure tree-shaken if blocks correctly hydrate

[trueadm]

Fixes #14588. We need to ensure we move the hydration node when the if block basically all gets tree-shaken away. Unfortunately, I found no way to test this as this relies on Rollup treeshaking away parts of the bundle and runtime.

[changeset-bot]

🦋 Changeset detected

Latest commit: 063ce36

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@14589

[Rich-Harris]

Not sure I understand how treeshaking can affect the behaviour?

[trueadm]

Not sure I understand how treeshaking can affect the behaviour?

Rollup is being super smart here, but it see's that the fn passed to $.if is empty, so it removes the function entirely. It then removes all the logic inside $.if relating to creation of the branches because of it, meaning we never move the hydration node.

Ending up with this:

function if_block(node, fn, elseif = false) {
  if (hydrating) {
    hydrate_next();
  }
  var anchor = node;
  var condition = null;
  var flags = elseif ? EFFECT_TRANSPARENT : 0;
  var has_branch = false;
  const update_branch = (new_condition, fn2) => {
    if (condition === (condition = new_condition)) return;
    let mismatch = false;
    if (hydrating) {
      const is_else = (
        /** @type {Comment} */
        anchor.data === HYDRATION_START_ELSE
      );
      if (condition === is_else) {
        anchor = traverse_nodes(true);
        set_hydrate_node(anchor);
        set_hydrating(false);
        mismatch = true;
      }
    }
    if (mismatch) {
      set_hydrating(true);
    }
  };
  block(() => {
    has_branch = false;
    if (!has_branch) {
      update_branch(null);
    }
  }, flags);
  if (hydrating) {
    anchor = hydrate_node;
  }
}

[Rich-Harris]

preview: https://svelte-dev-git-preview-svelte-14589-svelte.vercel.app/

this is an automated message

[Rich-Harris]

If the behaviour is different with treeshaking vs without then it must be a bug in Rollup, surely?

It's suspicious that we're calling update_branch(null) when condition is already null — means we're exiting early without ever hitting the if (hydrating) block. Could it be that we just need to do var condition = UNINITIALIZED?

[Rich-Harris]

(though I don't really understand why that wouldn't show up in our test suite already...)

[trueadm]

If the behaviour is different with treeshaking vs without then it must be a bug in Rollup, surely?

I thought that at first, but it's not because the entire block gets tree shaken, including the bit that does the hydration of the template – so we just need to ensure we move the hydration node accordingly.

[Rich-Harris]

Reproduced the bug locally in the sandbox, which confirms that it's nothing to do with treeshaking, which is a relief because that would have indicated a very serious bug in Rollup.

The answer is quite simple — per #14589 (comment), we just need to do var condition = UNINITIALIZED and then coerce condition to a boolean when checking for a hydration mismatch. Opened #14597