chore: Reduce hydration comment for {:else if} #15250

adiguba · 2025-02-09T12:56:34Z

Using {#if} {:else if} {/if} in template can produce lots of hydration code, since each {#if} or {:else if} will use a new block (with 2 possible hydration comments).

Example in this issue : #15200

I modified if_block() in order to handle that differently, using a single block for all if/else if.

Take example of this dummy code :

{#if prop === 1}
	<div>one</div>
{:else if prop === 2}
	<div>two</div>
{:else if prop === 3}
	<div>three</div>
{:else if prop === 4}
	<div>four</div>
{:else if prop === 5}
	<div>five</div>
{:else}
	<div>...</div>
{/if}

Currently, this can produces :

if prop === 1: <div>one</div>
if prop === 2: <div>two</div>
if prop === 3: <div>three</div>
if prop === 4: <div>four</div>
if prop === 5: <div>five</div>
else : <div>...</div>

With this change, it will produce only one hydration block in all case :

if prop === 1: <div>one</div>
if prop === 2: <div>two</div>
if prop === 3: <div>three</div>
if prop === 4: <div>four</div>
if prop === 5: <div>five</div>
if else : <div>...</div>

The generated code is also reduced and simplified.
On client side the generated code is :

{
    var consequent = ($$anchor) => {
      var div = root_1();

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

    var alternate_4 = ($$anchor) => {
      var fragment_1 = $.comment();
      var node_1 = $.first_child(fragment_1);

      {
        var consequent_1 = ($$anchor) => {
          var div_1 = root_3();

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

        var alternate_3 = ($$anchor) => {
          var fragment_2 = $.comment();
          var node_2 = $.first_child(fragment_2);

          {
            var consequent_2 = ($$anchor) => {
              var div_2 = root_5();

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

            var alternate_2 = ($$anchor) => {
              var fragment_3 = $.comment();
              var node_3 = $.first_child(fragment_3);

              {
                var consequent_3 = ($$anchor) => {
                  var div_3 = root_7();

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

                var alternate_1 = ($$anchor) => {
                  var fragment_4 = $.comment();
                  var node_4 = $.first_child(fragment_4);

                  {
                    var consequent_4 = ($$anchor) => {
                      var div_4 = root_9();

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

                    var alternate = ($$anchor) => {
                      var div_5 = root_10();

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

                    $.if(
                      node_4,
                      ($$render) => {
                        if ($$props.prop === 5) $$render(consequent_4); else $$render(alternate, false);
                      },
                      true
                    );
                  }

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

                $.if(
                  node_3,
                  ($$render) => {
                    if ($$props.prop === 4) $$render(consequent_3); else $$render(alternate_1, false);
                  },
                  true
                );
              }

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

            $.if(
              node_2,
              ($$render) => {
                if ($$props.prop === 3) $$render(consequent_2); else $$render(alternate_2, false);
              },
              true
            );
          }

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

        $.if(
          node_1,
          ($$render) => {
            if ($$props.prop === 2) $$render(consequent_1); else $$render(alternate_3, false);
          },
          true
        );
      }

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

    $.if(node, ($$render) => {
      if ($$props.prop === 1) $$render(consequent); else $$render(alternate_4, false);
    });
  }

This will become :

  var consequent = ($$anchor) => {
    var div = root_1();

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

  var alternate = ($$anchor, $$elseif) => {
    var consequent_1 = ($$anchor) => {
      var div_1 = root_3();

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

    var alternate_1 = ($$anchor, $$elseif) => {
      var consequent_2 = ($$anchor) => {
        var div_2 = root_5();

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

      var alternate_2 = ($$anchor, $$elseif) => {
        var consequent_3 = ($$anchor) => {
          var div_3 = root_7();

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

        var alternate_3 = ($$anchor, $$elseif) => {
          var consequent_4 = ($$anchor) => {
            var div_4 = root_9();

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

          var alternate_4 = ($$anchor) => {
            var div_5 = root_10();

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

          $.if(
            $$anchor,
            ($$render) => {
              if (prop() === 5) $$render(consequent_4); else $$render(alternate_4, false);
            },
            $$elseif
          );
        };

        $.if(
          $$anchor,
          ($$render) => {
            if (prop() === 4) $$render(consequent_3); else $$render(alternate_3, false);
          },
          $$elseif
        );
      };

      $.if(
        $$anchor,
        ($$render) => {
          if (prop() === 3) $$render(consequent_2); else $$render(alternate_2, false);
        },
        $$elseif
      );
    };

    $.if(
      $$anchor,
      ($$render) => {
        if (prop() === 2) $$render(consequent_1); else $$render(alternate_1, false);
      },
      $$elseif
    );
  };

  $.if(node, ($$render) => {
    if (prop() === 1) $$render(consequent); else $$render(alternate, false);
  });

Note : I also removed the JS scope block { ... }. I think this is useless...

Same thing on server side, where this code :

if (prop === 1) {
    $$payload.out += '<!--[-->';
    $$payload.out += `<div>one</div>`;
  } else {
    $$payload.out += '<!--[!-->';

    if (prop === 2) {
      $$payload.out += '<!--[-->';
      $$payload.out += `<div>two</div>`;
    } else {
      $$payload.out += '<!--[!-->';

      if (prop === 3) {
        $$payload.out += '<!--[-->';
        $$payload.out += `<div>three</div>`;
      } else {
        $$payload.out += '<!--[!-->';

        if (prop === 4) {
          $$payload.out += '<!--[-->';
          $$payload.out += `<div>four</div>`;
        } else {
          $$payload.out += '<!--[!-->';

          if (prop === 5) {
            $$payload.out += '<!--[-->';
            $$payload.out += `<div>five</div>`;
          } else {
            $$payload.out += '<!--[!-->';
            $$payload.out += `<div>...</div>`;
          }

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

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

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

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

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

will become :

  if (prop === 1) {
    $$payload.out += "<!--[-->";
    $$payload.out += `<div>one</div>`;
  } else if (prop === 2) {
    $$payload.out += "<!--[1-->";
    $$payload.out += `<div>two</div>`;
  } else if (prop === 3) {
    $$payload.out += "<!--[2-->";
    $$payload.out += `<div>three</div>`;
  } else if (prop === 4) {
    $$payload.out += "<!--[3-->";
    $$payload.out += `<div>four</div>`;
  } else if (prop === 5) {
    $$payload.out += "<!--[4-->";
    $$payload.out += `<div>five</div>`;
  } else {
    $$payload.out += "<!--[!-->";
    $$payload.out += `<div>...</div>`;
  }
  $$payload.out += `<!--]-->`;

Before submitting the PR, please make sure you do the following

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
Prefix your PR title with feat:, fix:, chore:, or docs:.
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.
If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

Run the tests with pnpm test and lint the project with pnpm lint

changeset-bot · 2025-02-09T12:56:37Z

🦋 Changeset detected

Latest commit: 1d5bedb

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

This PR includes changesets to release 1 package

Name	Type
svelte	Minor

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 · 2025-02-09T12:57:54Z

Playground

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

adiguba · 2025-02-09T13:45:34Z

Some little change so I edited the first message.

And a question : since if_block()'s signature has changed it's incompatible with previous version.

-if_block(..., elseif: boolean)
+if_block(..., elseif: number[])

I'm not sure if this is OK, or if we have to deal with a compatibility layer.

paoloricciuti · 2025-02-09T13:51:36Z

I'm not sure if this is OK, or if we have to deal with a compatibility layer.

Compiler output is not subject to semver so it's fine

trueadm · 2025-02-11T20:18:40Z

packages/svelte/src/internal/client/dom/blocks/if.js


-			if (!!condition === is_else) {
+			if (!!condition === is_else || isNaN(hydrate_index)) {


We definitely don't want to be using isNaN anywhere, it's a major codesmell

I didn't known that.
What the alternative to detect bad hydration comment ?

Validate it before doing parseInt?

What about hydrate_index !== hydrate_index, which will be true only for NaN ?

I mean you also have to facilitate mismatches, where the SSR content is intentionally different from the client side logic too, right? So in that case wouldn't the indexes also not match up?

Yes in this case the indexes will not match.

Here it is rather the case where the comment is not from the Svelte hydration process...

Also don't fully understand what the problem here is - isn't this more straightforward/less code to just go "this isn't a number, so something else went really wrong"?

isNaN has been condemned to hell all over, you can research its history. I'd just rather avoid it and it's caveats where possible.

trueadm · 2025-02-11T20:20:36Z

Note : I also removed the JS scope block { ... }. I think this is useless...

It wasn't useless, it was done intentionally to clearly show what blocks belong to the if block.

adiguba · 2025-02-11T20:42:38Z

I didn't think the generated code had to be so explicit. I will restore it...

trueadm

This is awesome, thank you.

trueadm · 2025-02-27T09:47:06Z

It needs a changeset though, and is probably a minor given it's quite a big change.

svelte-docs-bot · 2025-02-27T15:09:35Z

Preview: https://svelte-dev-git-preview-svelte-15250-svelte.vercel.app/

merge

adiguba · 2025-02-27T15:13:03Z

Nice,

I added a chanset and merged in order to launch the test with the last version...

rewrite else/if hydration

c9a8e22

adiguba added 2 commits February 9, 2025 14:20

fix: bad index

b13c419

reduce args on if_block()

4bf1f91

trueadm reviewed Feb 11, 2025

View reviewed changes

adiguba added 2 commits February 11, 2025 21:57

restore the block

d9fe35b

don't use isNan()

b3564e3

trueadm approved these changes Feb 27, 2025

View reviewed changes

changeset

157950e

Merge pull request #8 from sveltejs/main

1d5bedb

merge

trueadm merged commit 2032049 into sveltejs:main Mar 3, 2025
9 checks passed

github-actions bot mentioned this pull request Mar 3, 2025

Version Packages #15402

Merged

hobbes7878 mentioned this pull request Mar 27, 2025

Hydration error on EndNotes past [email protected] reuters-graphics/bluprint_graphics-kit#177

Closed


		if (!!condition === is_else) {
		if (!!condition === is_else \|\| isNaN(hydrate_index)) {

Uh oh!

chore: Reduce hydration comment for {:else if} #15250

chore: Reduce hydration comment for {:else if} #15250

Uh oh!

Conversation

adiguba commented Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before submitting the PR, please make sure you do the following

Tests and linting

Uh oh!

changeset-bot bot commented Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Feb 9, 2025

Uh oh!

adiguba commented Feb 9, 2025

Uh oh!

paoloricciuti commented Feb 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trueadm commented Feb 11, 2025

Uh oh!

adiguba commented Feb 11, 2025

Uh oh!

trueadm left a comment

Choose a reason for hiding this comment

Uh oh!

trueadm commented Feb 27, 2025

Uh oh!

svelte-docs-bot bot commented Feb 27, 2025

Uh oh!

adiguba commented Feb 27, 2025

Uh oh!

Uh oh!

Uh oh!

adiguba commented Feb 9, 2025 •

edited

Loading

changeset-bot bot commented Feb 9, 2025 •

edited

Loading