-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: more efficient template effect grouping #15050
Conversation
🦋 Changeset detectedLatest commit: 34ad652 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 |
preview: https://svelte-dev-git-preview-svelte-15050-svelte.vercel.app/ this is an automated message |
|
'1: $effect.pre 0', | ||
'1: render 0', | ||
'2: $effect.pre 0', | ||
'2: render 0', | ||
'3: $effect.pre 0', | ||
'3: render 0', | ||
'parent: render 0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the parent template now happening before all the children templates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the opposite. Previously you could have multiple template effects in the root fragment, and the {logRender()}
call happened before rendering the child. Now, there's just one template effect, at the end (after rendering child blocks and components etc). I don't consider this a breaking change since we're already grouping some effects at the end without giving you any control over it, and since template effects should be pure it's effectively not observable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This simplifies the compiler code around template effects and makes the output more readable and concise. Currently we often generate a bunch of template effects per fragment, when in almost all cases we need zero or one.
It also deduplicates repeated expressions, since there's no point running a pure function twice in the same moment.
A handful of tests needed to be updated because this changes the render order in a way that doesn't matter (i.e. text nodes and attributes in a given fragment are always updated at the end, even if they come before children in the DOM).
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 test
and lint the project withpnpm lint