chore: Reduce hydration comment for {:else if}

[adiguba]

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: <!--[1--><div>two</div><!--]-->
• if prop === 3: <!--[2--><div>three</div><!--]-->
• if prop === 4: <!--[3--><div>four</div><!--]-->
• if prop === 5: <!--[4--><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]

⚠️ No Changeset found

Latest commit: b3564e3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[github-actions]

Playground

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

[adiguba]

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]

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]

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

[adiguba]

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

[trueadm]

Validate it before doing parseInt?

[adiguba]

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

[trueadm]

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?

[adiguba]

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...

[dummdidumm]

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"?

[trueadm]

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]

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]

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