svelte

Commit Graph

Author	SHA1	Message	Date
Rich Harris	5122936edb	fix: treat batches as a linked list (#18205 ) This is another attempt to address some of the tricky edge cases that arise when async batches resolve out of order. The idea is this: Essentially, when a batch resolves: 1. we find the latest batch it shares changes with 2. if none exists (either because this is the earliest batch, or because it is independent of any earlier batches): - we commit it: effects are flushed, `oncommit` callbacks are run - we restart any async work in later batches that depends on both the committed values and the later batch's changes 3. otherwise, we don't commit the batch. instead: - we merge the changes from the later batch onto the earlier batch. in some cases this may mean restarting async work on the earlier batch, but we avoid doing so unnecessarily. - we then `#process()` the earlier batch. if it resolves, goto 1 This feels like it ought to work. There are still two failing tests, which I'm currently looking into. Notable changes: - Instead of having a `batches` set, we have a linked list. When a batch resolves, this makes it easy to find the batch that it should be merged into - The `#is_deferred` logic now takes account of skipped effects — there's no need to wait for a promise inside a falsy `if` block (this accounts for the change in the `async-inner-after-outer` test) - We no longer care about `#blockers` I would like to believe that this approach will allow us to simplify and delete some code, for example the `rebase` logic, though that remains to be seen. It also feels like some version of #18035 would be helpful. - closes #18189 - closes #18162 - fixes https://github.com/sveltejs/kit/issues/15431 ### 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. - [x] 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` --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>	3 months ago

Author

SHA1

Message

Date

Rich Harris

5122936edb

fix: treat batches as a linked list (#18205 )

This is another attempt to address some of the tricky edge cases that
arise when async batches resolve out of order. The idea is this:

Essentially, when a batch resolves:

1. we find the latest batch it shares changes with
2. if none exists (either because this is the earliest batch, or because
it is independent of any earlier batches):
    - we commit it: effects are flushed, `oncommit` callbacks are run
- we restart any async work in later batches that depends on both the
committed values and the later batch's changes
3. otherwise, we don't commit the batch. instead:
- we merge the changes from the later batch onto the earlier batch. in
some cases this may mean restarting async work on the earlier batch, but
we avoid doing so unnecessarily.
    - we then `#process()` the earlier batch. if it resolves, goto 1

This feels like it ought to work. There are still two failing tests,
which I'm currently looking into.

Notable changes:

- Instead of having a `batches` set, we have a linked list. When a batch
resolves, this makes it easy to find the batch that it should be merged
into
- The `#is_deferred` logic now takes account of skipped effects —
there's no need to wait for a promise inside a falsy `if` block (this
accounts for the change in the `async-inner-after-outer` test)
- We no longer care about `#blockers`

I would like to believe that this approach will allow us to simplify and
delete some code, for example the `rebase` logic, though that remains to
be seen. It also feels like some version of #18035 would be helpful.

- closes #18189
- closes #18162
- fixes https://github.com/sveltejs/kit/issues/15431

### 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.
- [x] 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`

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>

1 Commits (remove-was-marked)