perf: optimize non-reactive if statements

[benmccann]

first part of #13232

I only covered some simple cases for now as I'm still relatively new to working on the compiler so wanted to take baby steps

A couple things to check:

• was it right to add var $$anchor = node; inside the if?
• should I be pushing the <!> hydration marker in this case? I left the code doing that only for {#if}

[changeset-bot]

🦋 Changeset detected

Latest commit: 63cc21c

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

[Rich-Harris]

Gah — please don't force push unless it's absolutely essential. I was working on this branch locally and now I have to fight git

[Rich-Harris]

was it right to add var $$anchor = node; inside the if?

No, it needs to be let

should I be pushing the <!> hydration marker in this case?

It's not a hydration marker, it's only used when not hydrating, and yes it's still necessary

[Rich-Harris]

Made it work in more cases (like {#if true}) by using the existing expression metadata mechanism. It's not enough though. We need to handle hydration mismatches in cases like these:

{#if typeof window !== 'undefined'}
  <span>browser stuff</span>
{/if}

So instead of code like this...

{
  let $$anchor = node;

  $.next();

  if (typeof window !== 'undefined') {
    var span = root_1();

    $.append($$anchor, span);
  }
}

...we would need to generate code like this...

{
  let $$anchor = node;
  let $$condition = typeof window !== 'undefined';
  let $$mismatch = $.hydrate_if($$anchor, $$condition);

  if ($$condition) {
    var span = root_1();

    $.append($$anchor, span);
  }

  if ($$mismatch) {
    $.set_hydrating(true);
  }
}

...where hydrate_if looks something like this (we could, at least, use it inside if.js):

function hydrate_if(anchor, condition) {
  if (!hydrating) return false;

  const is_else = /** @type {Comment} */ (anchor).data === HYDRATION_START_ELSE;

  if (condition === is_else) {
    // Hydration mismatch: remove everything inside the anchor and start fresh.
    // This could happen with `{#if browser}...{/if}`, for example
    anchor = remove_nodes();

    set_hydrate_node(anchor);
    set_hydrating(false);
    return true;
  }

  return false;
}

This is obviously quite a bit more code than the status quo...

$.if(node, () => typeof window !== 'undefined', ($$anchor) => {
  var span = root_1();

  $.append($$anchor, span);
});

...and it won't always be treeshaken away. So there is a trade-off involved here.

One other detail: this technically breaks transitions. Today, if you have something like this...

{#if foo}
  {#if true}
    <div transition:fade>...</div>
  {/if}
{/if}

...then the transition won't play when foo changes, because it's local to the inner block. Obviously that wouldn't be very useful in the first place, but there probably could be a case where it mattered, and it would behave differently as of this PR because no effect is being created for the if true block. Not sure how to address that.

[Rich-Harris]

requesting changes so we don't merge this until hydration mismatches are solved

[benmccann]

Hmm, perhaps we should close this then. My original goal was to shrink the code output, but we'd be going the other direction here.

I do still wonder about #each though. Encountering a non-reactive #if isn't very common, but non-reactive #each blocks are very common. About 1/4 of them on svelte.dev fall into that category (though we often get the data through an import which would require cross-file compilation to really benefit unless we manually reassigned the imported data to a const or moved it to live in the .svelte file instead of its own file).

Here's an example:

svelte/sites/svelte.dev/src/routes/_components/Demo.svelte

Line 5 in ed7611b

const examples = [

Would we run into the same degree of difficult there? Like in that code I think we could determine that examples would be the same on the server and browser, which might save us the need to do some hydration checks. But you could have an #if browser check inside the loop. I don't know where that would land us and if there's much of an opportunity for a non-reactive #each?

[Rich-Harris]

What would be the benefit? I get why we'd want to do this for if blocks — dead code elimination — but that doesn't apply for each blocks

[benmccann]

A standard for could potentially be less bytes (if we don't have to add extra hydration handling code) and would also execute faster

[Rich-Harris]

What would the generated code be, for something like this for example?

{#each 'abc' as letter}
  <span>{letter}</span>
{/each}

Today it's this:

$.each(node, 0, () => 'abc', $.index, ($$anchor, letter) => {
  var span = root_1();
  var text = $.child(span);

  $.reset(span);
  $.template_effect(() => $.set_text(text, letter));
  $.append($$anchor, span);
});

Bear in mind it needs to handle hydration (including mismatches), and things like {#each nullish ...} and {:else} blocks in addition to the above. I'm not sure I see where the savings would come from. And we'd still have the aforementioned issue around transition locality

[benmccann]

Ok, well I'm going to go ahead and close this then