chore: very minor perf improvement when fallback values are used

[benmccann]

From this discussion #11842 (comment) it seems best for performance to avoid having unnecessary async functions

This is a pretty minimal change, so feel free to close it if it's not worth it. But I noticed while investigating that this was the only place in the codebase where this occurs, so I figured I might as well update it

[changeset-bot]

⚠️ No Changeset found

Latest commit: cdf5808

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

[dummdidumm]

I vaguely remember having added this due to some code transformations. Now that this function looks exactly like its synchronous counterpart we can just remove it in favor of the sync one.

[Rich-Harris]

I'm trying to wrap my head round whether we need any of this. By commenting stuff out and seeing which tests fail, I see that this...

let value = $state(0);

const update = async () => {
  ({ value = await Promise.resolve(1) } = { value: 2 });
}

...results in this:

const update = async (_, value) => {
  await (async () => {
    const tmp = { value: 2 };

    (
      $.set(value, $.proxy(await $.value_or_fallback_async(tmp.value, async () => await Promise.resolve(1))))
    );

    return tmp;
  })();
};

Why isn't it just this?

const update = async (_, value) => {
  const tmp = { value: 2 };

  $.set(value, $.proxy(tmp.value !== undefined ? tmp.value : await Promise.resolve(1)))
};

Are there cases where we really need all that machinery?

[dummdidumm]

Sadly yes:

let value = $state(0);

const update = async () => {
	let x = { foo: ({ value = await Promise.resolve(1), x = await value } = { value: 2 })};
}

[Rich-Harris]

99% of AssignmentExpression nodes are the direct children of ExpressionStatement nodes — in other words, we can ditch the function wrapper in the vast majority of cases.

For the edge cases that remain, I'm pretty sure we can still make things a lot simpler.

input:

<script>
  let value = $state(0);

  const update = async () => {
    ({ value = await one() } = { value: await two() });
    
    let x = {
      foo: ({ value = await three() } = { value: await four() })
    };
  }
</script>

output:

import * as $ from "svelte/internal/client";

export default function App($$anchor) {
  let value = $.source(0);

  const update = async () => {
    await (async () => {
      const tmp = { value: await two() };

      (
        $.set(value, $.proxy(await $.value_or_fallback_async(tmp.value, async () => await one())))
      );

      return tmp;
    })();

    let x = {
      foo: await (async () => {
        const tmp_1 = { value: await four() };

        (
          $.set(value, $.proxy(await $.value_or_fallback_async(tmp_1.value, async () => await three())))
        );

        return tmp_1;
      })()
    };
  };
}

better output (three fewer async functions, much less code):

import * as $ from "svelte/internal/client";

export default function App($$anchor) {
  let value = $.source(0);

  const update = async () => {
    const tmp = { value: await two() };

    $.set(value, $.proxy(tmp.value !== undefined ? tmp.value : await one()));

    let x = {
      foo: await (async (tmp_1) => {
        $.set(value, $.proxy(tmp.value !== undefined ? tmp.value : await three()));

        return tmp_1;
      })({ value: await four() })
    };
  };
}

[dummdidumm]

Should we postpone this to 5.x? Feels like a micro optimization we can do later any time.

[Rich-Harris]

Yes, we should refactor serialize_set_binding first anyway

[Rich-Harris]

After #12614, #12780 and #12788 the situation is much improved — closing