fix: reduce if block nesting #17662
Open
fix: reduce if block nesting #17662
+656
−55
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reduces if block nesting similar to how we did it in #15250 (which got lost during the `await` feature introduction): If the if expression doesn't contain an await expression or is not dependent on a blocker that is not already resolved, then we can avoid creating a separate `$.if()` statement. The one trade-off is that we'll do re-invocations for all the conditions leading up to the condition that matches. Therefore non-simple if expressions are wrapper in `$.derived` to avoid excessive recomputations. closes #17659 (~320 markers in prod mode possible now; less in dev because of our "wrap this component with devtime info" method) helps with #15200
🦋 Changeset detectedLatest commit: dd221fc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
dummdidumm
temporarily deployed
to
Publish pkg.pr.new (maintainers)
GitHub Actions
Inactive
Contributor
|
dummdidumm
temporarily deployed
to
Publish pkg.pr.new (maintainers)
GitHub Actions
Inactive
Rich-Harris
reviewed
Feb 9, 2026
| } | ||
| } | ||
|
|
||
| const render_call = b.stmt(b.call('$$render', consequent_id, b.literal(index))); |
Member
There was a problem hiding this comment.
if we give this argument a default value we can generate smaller code in the common case where there's no else-if
Suggested change
| const render_call = b.stmt(b.call('$$render', consequent_id, b.literal(index))); | |
| const render_call = b.stmt(b.call('$$render', consequent_id, index > 0 && b.literal(index))); |
| * @param {null | ((anchor: Node) => void)} fn | ||
| */ | ||
| function update_branch(condition, fn) { | ||
| function update_branch(key, fn) { |
Member
There was a problem hiding this comment.
Suggested change
| function update_branch(key, fn) { | |
| function update_branch(key = 0, fn) { |
Rich-Harris
reviewed
Feb 9, 2026
Comment on lines
+45
to
+53
| // Wrap complex expressions (anything beyond a simple identifier) in $.derived() for memoization. | ||
| // TODO check expression content more thoroughly to avoid wrapping for stuff like `foo > 1` or `foo.length` | ||
| if ( | ||
| branch.test.type !== 'Identifier' && | ||
| branch.test.type !== 'Literal' && | ||
| (branch.test.type !== 'MemberExpression' || | ||
| // foo.bar is fine but foo[bar] is not | ||
| branch.metadata.expression.dependencies.size > 1) | ||
| ) { |
Member
There was a problem hiding this comment.
In analogous situations we use this check instead:
Suggested change
| // Wrap complex expressions (anything beyond a simple identifier) in $.derived() for memoization. | |
| // TODO check expression content more thoroughly to avoid wrapping for stuff like `foo > 1` or `foo.length` | |
| if ( | |
| branch.test.type !== 'Identifier' && | |
| branch.test.type !== 'Literal' && | |
| (branch.test.type !== 'MemberExpression' || | |
| // foo.bar is fine but foo[bar] is not | |
| branch.metadata.expression.dependencies.size > 1) | |
| ) { | |
| if (branch.metadata.expression.has_call) { |
For example a component like this
<Foo x={a.b.c} y={fn()} />yields this:
{
let $0 = $.derived(() => $$props.fn());
Foo($$anchor, {
get x() {
return $$props.a.b.c;
},
get y() {
return $.get($0);
}
});
}I think that's probably better even if it means there's a possibility of additional computation in some cases — no need to create deriveds here, for example:
{#if color === 'red'}
<p>red</p>
{:else if color === 'blue'}
<p>blue</p>
{:else if color === 'green'}
<p>green</p>
{/if}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reduces if block nesting similar to how we did it in #15250 (which got lost during the
awaitfeature introduction): If the if expression doesn't contain an await expression or is not dependent on a blocker that is not already resolved, then we can avoid creating a separate$.if()statement. The one trade-off is that we'll do re-invocations for all the conditions leading up to the condition that matches. Therefore non-simple if expressions are wrapper in$.derivedto avoid excessive recomputations.closes #17659 (~320 markers in prod mode possible now; less in dev because of our "wrap this component with devtime info" method) helps with #15200
Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint