chore: null out current_batch before committing branches (#17809)

Another small tweak extracted from #17805, just to make that diff a bit
more legible.

By passing the `batch` to the branch commit callback, we don't need to
rely on the value of `current_batch` being the same as the batch
currently being processed. That gives us more control over the order of
operations — for example we can null out `current_batch` _before_
committing branches, which is important (at present, if a state change
occurs while those branches are being committed, it will belong to the
current batch, but the resulting effects will happen in the context of a
_new_ batch, which is something we need to avoid for the sake of
#17805).

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

- [x] 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
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [ ] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`
pull/17814/head
Rich Harris 7 months ago committed by GitHub
parent fa4f1c45f4
commit b76cd5cafc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
chore: null out current_batch before committing branches

@ -68,9 +68,10 @@ export class BranchManager {
this.#transition = transition; this.#transition = transition;
} }
#commit = () => { /**
var batch = /** @type {Batch} */ (current_batch); * @param {Batch} batch
*/
#commit = (batch) => {
// if this batch was made obsolete, bail // if this batch was made obsolete, bail
if (!this.#batches.has(batch)) return; if (!this.#batches.has(batch)) return;
@ -221,7 +222,7 @@ export class BranchManager {
this.anchor = hydrate_node; this.anchor = hydrate_node;
} }
this.#commit(); this.#commit(batch);
} }
} }
} }

@ -87,7 +87,7 @@ export class Batch {
/** /**
* When the batch is committed (and the DOM is updated), we need to remove old branches * When the batch is committed (and the DOM is updated), we need to remove old branches
* and append new ones by calling the functions added inside (if/each/key/etc) blocks * and append new ones by calling the functions added inside (if/each/key/etc) blocks
* @type {Set<() => void>} * @type {Set<(batch: Batch) => void>}
*/ */
#commit_callbacks = new Set(); #commit_callbacks = new Set();
@ -207,19 +207,19 @@ export class Batch {
reset_branch(e, t); reset_branch(e, t);
} }
} else { } else {
// If sources are written to, then work needs to happen in a separate batch, else prior sources would be mixed with
// newly updated sources, which could lead to infinite loops when effects run over and over again.
previous_batch = this;
current_batch = null;
// append/remove branches // append/remove branches
for (const fn of this.#commit_callbacks) fn(); for (const fn of this.#commit_callbacks) fn(this);
this.#commit_callbacks.clear(); this.#commit_callbacks.clear();
if (this.#pending === 0) { if (this.#pending === 0) {
this.#commit(); this.#commit();
} }
// If sources are written to, then work needs to happen in a separate batch, else prior sources would be mixed with
// newly updated sources, which could lead to infinite loops when effects run over and over again.
previous_batch = this;
current_batch = null;
flush_queued_effects(render_effects); flush_queued_effects(render_effects);
flush_queued_effects(effects); flush_queued_effects(effects);
@ -358,6 +358,7 @@ export class Batch {
if (batches.size > 1) { if (batches.size > 1) {
this.previous.clear(); this.previous.clear();
var previous_batch = current_batch;
var previous_batch_values = batch_values; var previous_batch_values = batch_values;
var is_earlier = true; var is_earlier = true;
@ -421,7 +422,7 @@ export class Batch {
} }
} }
current_batch = null; current_batch = previous_batch;
batch_values = previous_batch_values; batch_values = previous_batch_values;
} }
@ -479,7 +480,7 @@ export class Batch {
this.flush(); this.flush();
} }
/** @param {() => void} fn */ /** @param {(batch: Batch) => void} fn */
oncommit(fn) { oncommit(fn) {
this.#commit_callbacks.add(fn); this.#commit_callbacks.add(fn);
} }

Loading…
Cancel
Save