Merged #18298 a bit too soon - there's still a situation which can
corrput the linked list: If we merge one batch into another, it will
unlink the batch immediately. But `discard` also calls it, running the
unlink logic again, which can corrupt the linked list.
Sketch:
1. Batch A is pending.
2. Batch B starts later, intersects A, resolves first, and merges into
A.
3. B is unlinked immediately, but `A.oncommit(() => B.discard())`
remains.
4. Independent batch C is created while A is still pending and is linked
after A.
5. A commits, calls `B.discard()`, and B's stale `#prev`/`#next` can set
`A.#next = null` / `last_batch = A`, disconnecting C.
Not able to produce a failing test from it but it's definitely a fix we
need to make.
Also moved `#link` into a constructor, because it is (and should be)
used only once.
Also made the action after an error `discard` instead of just `#unlink`
because this batch is done for and e.g. pending `settled` should
resolve, too.
We incorrectly restore effect context after an`$derived(await ...)` if
it occurs inside an async function, but only for those, no other `await`
expressions. This is buggy and wrong. We only want to restore context
_inside_ an async derived expression (including template expressions)`
so that `await a + b` works correctly.
It's possible that this will be a breaking change (albeit not
semver-violating, because `experimental`) for some people if they are
creating effects after an `await` (other than at the top level of a
component). This is regrettable, but it should never have worked in the
first place.
Right now, if you have the following:
```svelte
{@const data = await foo}
<p>{(() => data)()}<p>
```
It will blow up during CSR, because it tries to read `data` before it
exists. The solution to this is to consider references inside closures
as blockers for those closures. This _does_ mean we'll overblock in some
circumstances, such as:
```svelte
{@const data = await foo}
<button onclick={() => data}>foo<button>
```
But I wonder if that's actually incorrect? If the user were to click
`onclick` before `data` is ready, it would blow up.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Follow-up to #18273 (not merged yet hence no changeset here): We can run
into a null-pointer when wanting to increment/decrement inside an effect
root that is outside a the component tree. Similarly, if not using the
component logic to unset context at the right time we gotta do it
"manually" inside `save`.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
## Summary
The `script_unknown_attribute` warning message used American English
(`Unrecognized`) while every other user-facing error/warning in the
compiler uses British English (`Unrecognised`):
- `options_unrecognised` error: `Unrecognised compiler option…`
- `unknown_code` warning: `is not a recognised code`
This change updates `script_unknown_attribute` to match, then
regenerates `warnings.js` and the reference documentation from the
source message file via `node scripts/process-messages`.
## Test plan
- Spelling change is isolated to the `script_unknown_attribute` warning
message
- `warnings.js` and generated docs were updated by running `node
scripts/process-messages`
- All three occurrences (source, generated JS, generated docs) are
consistent
If a branch is removed from the visible dom, it may be kept around
because a subsequent batch will intro it again. If we don't resume the
effects it will stay inert and therefore not react to updates anymore
Several constant lookup tables in `utils.js` were arrays searched with
`Array.prototype.includes`, which is O(n). They're queried often — per
attribute during attribute setup and SSR, per event during event
delegation, and per identifier during compilation. Switching them to
`Set` makes each lookup O(1) without changing any public behaviour.
Ref: https://github.com/sveltejs/svelte/issues/15100
Adds the tag name to the `a11y_click_events_have_key_events` warning
given when a non-interactive element has a click handler but no keyboard
events.
This already happens in `a11y_no_static_element_interactions`, and
should probably happen in many more messages.
Fixes#14413.
This keeps the temporary raw-text hydration sentinel used by dynamic
`<svelte:element>` from becoming part of the final DOM. Hydration still
gets a marker to advance through for raw-text children, but the marker
is removed after the child renderer runs, so `<style>` and `<script>`
contents stay identical to SSR output.
Tests:
- `pnpm test hydration -t svelte-head-dynamic-style`
- `pnpm test hydration`
- `pnpm test runtime-runes -t svelte-element`
- `pnpm lint`
- `git diff --check`
---------
Co-authored-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
## Summary
The propagation walk in `handle_event_propagation` already calls
`event.composedPath()` at the start to find the entry index, but then
re-derives the same chain step-by-step via `current_target.assignedSlot
|| current_target.parentNode || .host`. Three property reads per
iteration is measurable on the hot event path.
Walk the captured `path` array by index instead.
## Notes on behavior
`composedPath()` is the spec-compliant snapshot of the dispatch chain:
- Same shadow-DOM crossings (slots and shadow roots are included for
composed events).
- Same `host` traversal (composed-path crosses shadow boundaries when
appropriate).
- Differs from the previous walk in one edge case: if a handler removes
a parent mid-dispatch, the snapshot-based walk continues through the
captured chain (matches native browser semantics — the previous
`parentNode` walk would have stopped at a null parent).
## Performance
Measured in real Chromium on a click through a 30-deep tree with five
delegated handlers: **~245k hz → ~277k hz** (~+13%, ~−12% per-event
time).
## Test plan
- [x] All 6006 runtime tests pass (runtime-runes + runtime-legacy +
runtime-browser)
- [x] Native shadow-DOM event tests (in runtime-browser) pass unchanged
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A committing/merging batch can have promises that were rejected (e.g. as
obsolete). We gotta "forward" this rejection, too, instead of just the
successful promise. At best it results in a uncaught rejection
(`async-branch-merge-obsolete`), at worst it means error boundaries are
not correctly displayed (`async-later-promise-fails-first`).
Solves the reproduction in
https://github.com/sveltejs/svelte/issues/18221#issuecomment-4507803845
This fixes the issue in
https://github.com/sveltejs/svelte/issues/18221#issuecomment-4497918414
where an error can create follup-up invariant errors. The batch errors,
has no chance to run otherwise (no more pending work) and is therefore
"dead". That means we need to unlink it otherwise it's becoming a
"zombie" and hangs around, causing unnecessary and potentially buggy (as
seen in the reproduction) merge/commit work.
I was not able to reduce the reproduction down to a test case that fails
without the fix, but it does make a related error test from #17888 work
more correctly.
While looking at
https://github.com/sveltejs/svelte/issues/18221#issuecomment-4497918414
and trying to understand how the invariant can happen I noticed that we
are not correctly filtering during commit.
- we were not ignoring deriveds
- we were not comparing the correct values (checking `source.v` instead
of the saved value) and not checking if their "is a derived" state
differs
I'm not able to come up with a test where something fails without these
(possibly because it's more about an optimization to do less reruns and
not about correctness) fixes, but they do make sense.
While looking at the reproduction in
https://github.com/sveltejs/svelte/issues/18221#issuecomment-4497918414
I immediately got greeted with a runtime error when running it in the
playground (weirdly not in the Stackblitz version). The error was that a
component expected a binding to be set in onMount, but the timing of
onMount was wrong.
Turns out it's because our logic to determine whether or not to defer
top level effects is flawed. `REACTION_RAN`, which was used previously,
is already set if the initialized component is inside an async block. We
instead check for `component_context.i` which is set to `true` on
`pop()`.
An effect could be gated behind a branch. If we don't defer + transfer
them upon merge, the branch would still be marked clean but the effect
behind it is dirty but no longer reachable. It's not reachable via mark
either because that one only concerns itself with block/async effects,
and the branch gating the effect is not guaranteed to be touched by
that.
Fixes#18249
Little sad side-effect: Since we cannot reliably know _before_ traversal
if we have no blocking pending work left (the traversal could mark an if
block falsy which contains the last blocker), we gotta undo a
performance optimization.
Thanks to
https://github.com/sveltejs/svelte/issues/17940#issuecomment-4480016550
I was finally able to isolate and reproduce a false-positive invariant
error. I had a hunch this could happen and this shows it. Essentially,
you can end up in situations where two batches are scheduled to run in
the same microtask queue flush, and if the first rebases the second the
invariant will throw, which is wrong. We can avoid this by checking if a
decrement is queued.
This started out as an investigation to get rid of the runtime code for
`{#await ...}` that deactivated a batch prior to reading the promise
function. That can result in a new batch being created if the promise
invocation happens to write a source.
Through that I discovered two other bugs:
1. The way we handle `{#await await ...}` was flawed around
SSR/hydration: On the server it would await the expression (which it
should not; `{#await await ...}` is kind of a weird special case here)
and during hydration it did not produce matching nodes, leading to
hydration fails.
2. When reading a dependency after an await expression which we add to
the current reaction, we did not deduplicate those reads. That can lead
to duplicate dependencies, which in turn can lead to bugs when
`remove_reaction` later runs. Conside this: You have `deps = [count,
unrelated, count]`. Now you do `remove_reactions(deps, 1)`, i.e. "remove
all reactions after the first one". That means the "disconnect these
from each other" logic runs for `count`, too, because it's also in the
third position, but that is wrong because it is also in the first
position, i.e. the connection should be kept.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
### 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. Fixes#10031.
- [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`).
### What this changes
Server attribute template generation currently wraps each dynamic
expression in `$.stringify`, even when the compiler can prove the
expression is a string or a known constant. This reuses the existing
scope evaluation metadata so server output can avoid `$.stringify` for
proven string/constant chunks while keeping it for possibly nullish
unknown values.
The updated snapshot covers a mixed attribute with a known string,
mutable state, `null`, numeric/undefined constants, a known
string-producing `typeof`, and an unknown prop value.
### Tests and linting
- [x] `pnpm test snapshot -t nullish-coallescence-omittance`
- [x] `pnpm test snapshot`
- [x] `pnpm --filter svelte check`
- [x] `pnpm lint`
- [x] `pnpm prettier --check .changeset/slow-bikes-serve.md`
---------
Co-authored-by: Rich Harris <hello@rich-harris.dev>
Fixes#18206 and fixes#18207 — both are printer bugs in
`packages/svelte/src/compiler/print/index.js`.
## Changes
### Fix 1: `svelte:body` crashes the printer (#18206)
`SvelteBody` was missing from the visitor map, causing a crash with
`Error: Not implemented: SvelteBody`. Added the handler (same one-liner
pattern as `SvelteDocument`, `SvelteHead`, etc.) and added `SvelteBody`
to the `is_block_element` check so whitespace is handled consistently.
### Fix 2: Keyframe percent stops print as `0%%` (#18207)
`Percentage.value` already includes the `%` sign (captured by
`/\d+(\.\d+)?%/y`), but the printer was writing `` `${node.value}%` `` —
appending a second `%`. Changed to `context.write(node.value)` to match
the `Nth` printer pattern directly above it.
Also updated the existing `style` snapshot which had `50%%` (the bug was
silently baked in), and added dedicated test samples for both fixes.
This isn't _really_ a fix since these should never surface to the user,
but it's useful for debugging when they do, as in
https://github.com/sveltejs/kit/pull/15779. Instead of seeing `Symbol()`
we see e.g. `Symbol(uninitialized)` which makes it easier to understand
where a bug is coming from.
The logic was flawed - a teardown effect only has a teardown function
but not `fn` property, but unfreeze thought that everything with a
`teardown` needs to be unfreezed
Helps with #18221 (though likely doesn't fix it completely, at least not
the more general `batch.#roots`problems)
Turns out there are a few unavoidable cases where we have to execute the
derived even if we otherwise wouldn't, because of its lazy nature.
Fixes#18139
Our `run` function which executes top level awaits (and synchronous
statements in-between/after) did not unset the context in time in case
the function returns an async value. In that case the context was still
around until the that promise resolves, which can be too late because
unrelated things can be intertwined with the batch.
The test shows this: Without the fix, the unrelated count incrementation
would not update the view until the top level awaits in the child are
done. In the test this just shows as a delayed visual update, but it
also can result in stale roots as shown in
https://github.com/sveltejs/svelte/issues/18221#issuecomment-4470921077
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.
# Releases
## svelte@5.55.7
### Patch Changes
- fix: prevent XSS on `hydratable` from user contents
([`a16ebc67bbcf8f708360195687e1b2719463e1a4`](a16ebc67bb))
- chore: bump devalue
([#18219](https://github.com/sveltejs/svelte/pull/18219))
- fix: disallow empty attribute names during SSR
([`547853e2406a2147ad7fb5ffeba95b01bd9642da`](547853e240))
- fix: harden regex
([`d2375e2ebcab5c88feb5652f1a9d621b8f06b259`](d2375e2ebc))
- fix: move Svelte runtime properties to symbols
([`e1cbbd96441e82c9eb8a23a2903c0d06d3cda991`](e1cbbd9644))
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
#18108, with two differences:
- we use a global map
- we use the parent reaction as the key, rather than traversing upwards
for a branch
I think this has the same outcome?
### 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 Holthausen <simon.holthausen@vercel.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
One-line typo fix in
`packages/svelte/src/internal/client/dom/elements/events.js`: "removal
or moving of of the DOM" → "removal or moving of the DOM". No
code/behavior change.
---------
Co-authored-by: Kai Tanaka <275430420+quyentonndbs@users.noreply.github.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
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>
Realised that we can make the logic in `#process()` a little easier to
follow by early-returning if a batch is deferred (or blocked) after
processing, since nothing that happens after the `if` block applies in
that case
Noticed that we're not actually doing anything with `source_stacks` — we
shadow the module-level declaration in `flush`, which means we just keep
appending to it and then clearing a different (and empty) set. As a
result, any source that ever gets an `updated` property never gets rid
of it. This probably causes a memory leak?
Anyway, this fixes it.
### Before submitting the PR, please make sure you do the following
- [ ] 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`
Fixes a regression of #18170 (not released yet therefore no changeset).
`current_batch` is nulled out if the `#commit` rebases other branches,
and that can lead to nullpointers down the line.
No test right now but it's part of getting the failing SvelteKit test
passing.
This incorporates some of the fixes and insights from #18177, but gets
rid of the `skip` logic.
Instead, we differentiate between _stale_ and _obsolete_ promises. A
promise is stale if it has been overtaken by a subsequent update, and
was rejected with `STALE_REACTION`:
```ts
async function search(query: string) {
return fetch(`/search?q=${query}`, { signal: getAbortSignal() }).then((r) => r.json());
}
```
In this case, if we start typing `pot`, and then finish typing `potato`,
the first promise will eventually resolve with the results for
`/search?q=potato`, instead of the batch entering a weird limbo/zombie
state.
A promise is obsolete if it belongs to a now-destroyed effect, meaning
that toggling `show` doesn't result in an accumulation of
never-resolving batches:
```svelte
{#if show}
{await neverResolves()}
{/if}
```
Fixes part of https://github.com/sveltejs/kit/issues/15431
---------
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
We shouldn't continue executing async work where we know the surrounding
branch is destroyed already, it can leave to noisy "derived inter"
warnings or even runtime errors ("cannot stringify symbol" when running
a template effect with an uninitialized source). Neither should we warn
about waterfalls on an already-destroyed async effect.
Fixes#18097 (though strictly speaking that particular instance is also
fixed by #18117 which fixes the underlying cause for the reruns; this
one is necessary in itself though, as shown by the new test)
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
These might have been necessary at one point, but I'm confident they're
unnecessary now — `increment_pending` happens (if necessary) inside
`flatten`, which is called inside `async` and
`deferred_template_effect`, so there's no need to call it inside those
functions as well.
Don't have a test for it and no bug report but I stumbled upon this and
I'm very certain not restoring context here is wrong since it means the
failed snippet rendering gets the wrong context.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
This is a regression from #18117 - we moved `this.#commit()` higher up
but that means that `current_batch` could be nulled out / overridden
through `batch.activate/deactivate` / blocker runs inside `#commit()`.
Therefore restore the previous value afterwards. No changest because
#18117 is not released yet.
Fixes the other part of the failing SvelteKit `query.live` test.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
The fix in #17966 wasn't quite right, because we gotta rethrow in case
the iterator stopped because of an error. Fixes part of the SvelteKit
`query.live` test failure.
---------
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Closes#18168
Not sure if there's a deeper issue in play because the error it's only
really there if you also add a title to the component. I think the issue
is that with multiple arguments the top level `Promise.all` is not
wrapped in `save` and that probably causes a race condition with `title`
that sets the context back to `null` in a `finally`.
One issue is that now the generated code looks like this
```js
const [$$0, $$1] = (await $.save(Promise.all([
(async () => (await $.save(user()))().name)(),
(async () => (await $.save(user()))().image)()
])))();
```
which seems a bit redundant, but I'm not sure if we can get rid of the
inner `save` since they are indeed awaiting something.
While looking into #18162 I found an adjacent bug. Currently, if an
async derived resolves in batch 2 before it resolves in batch 1, we
reject the promise belonging to batch 1 and by extension the batch
itself. This means that any other changes in batch 1 are silently
discarded, incorrectly.
The fix is almost comically simple: rather than rejecting the earlier
promise, we just resolve it with the latest value.
I have a hunch that this might also enable us to simplify the rebase
logic, though I haven't investigated that in this PR.
### 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`
We had logic in place to ignore errors of `$inspect` effects that are
about to destroy, but we didn't take into account that we can get these
transient errors while checking for `is_dirty` in preparation for
running the effect, too. Now effects are marked as dirty in case an
error occurs while evaluating their dependencies, which guarantees we
will see the error again but we can then handle it properly.
Fixes#15741
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
While working on #18106 I noticed that we're not adding eager effects
inside `mark_reactions` when `DEV` is `false`. As a result production
build could have `$state.eager` or `$state.pending` not working
correctly.
~No test because I can't get Vitest to not run with `DEV` being `true`.~
added a test
It's possible to rebase just-created batches.
Case A:
- batch A runs effects
- one of these effects writes to a source. This creates a new batch B
- an effect _after_ that (still part of "flush effects of batch A")
executes a derived. This creates an entry in the `current` Map in batch
B
- batch A commits after processing batch B (`next_batch` etc logic),
batch B is pending. Due to derived being part of batchB.current batch A
can wrongfully think these are connected and try to rerun/add effects
etc on batch B
Case B:
- like case A but with an additional await inside a pending snippet
Case C:
- batch A with source a and b, it flushes effects
- one of these effects schedules batch B with b and c scheduling an
async effect
- batch B is deferred
- batch A commits. Due to the a/b/c partial overlap it will needlessly
rerun the just scheduled async effect
All these cases are wrong. We fix it like this:
1. we call `this.#commit()` _before_ running the new batches, which may
stick around due to having pending work, and we don't want to rebase
these. This fixes case A and C
2. we capture derived values in `previous_batch` if it exists, because
it means we're currently flushing effects, and derived writes belong to
that batch and not a new one that might have been scheduled already.
This fixes case B
Discovered this while working on #18097
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
## Summary
- Fix the `constructor` typo in the attributes comment.
## Related issue
- N/A
## Guideline alignment
- Read `CONTRIBUTING.md` and kept this to one focused text-only file
change.
- No behavior, test, fixture, changeset, or generated-file changes.
## Test plan
- `git diff --check`
- Not run: comment-only change.
Co-authored-by: Codex <codex@openai.com>
Fix#18132
This PR treat lazy fallbacks on `prop()` as derived. Now a default
function that uses a $state is recalculated whenever its dependents
changes. This change implies that this lazy functions cannot mutate a
state anymore (because it is derived), causing a
`state_unsafe_mutation`error. This implies on a breaking change, but
reasonable.
---
### New breaking change here
- **Who does this affect**: Everyone that has updated a $state on a
default lazy prop. Example:
```html
<script>
let myValue = $state(0);
let callCount = $state(0);
function getValue() {
callCount++; // causes a state_unsafe_mutation error
return myValue; // returning a state doesn't cause an error, and now it is tracked as a dependency
}
let { value = getValue() } = $props();
</script>
```
**Why make this breaking change**
This encourages people to not update states on a function that
fundamentaly, is readonly. When someone wants to use a default function
expecting that it should be tracked, its not likely that this function
will change some state. It is anti-pattern to change some state inside a
getter function.
But what if someone wants to do it, like in the code above?
The code above doesn't make sense before this PR, the old way to
calculate lazy functions is to execute it one time, and only one, so the
`callCount` variable will never change. But let's assume that someone
did it, how to migrate?
The migration in same example is easy, since the `callCount` is executed
only once, it will not be executed after the component is mounted. So
the `callCount` doesn't need to be a state, the `callCount` will be in a
valid state when the component is created. So here is the migrated code:
```html
<script>
let myValue = $state(0);
let callCount = 0;
function getValue() {
callCount++; // doesn't causes an error
return myValue;
}
let { value = getValue() } = $props();
</script>
```
As we can see, there is no reason for the variable `callCount` in this
example (before this PR), and if someone did it, it is more likely that
they used a constant instead:
```html
<script>
let myValue = $state(0);
let callCount = 1;
function getValue() {
return myValue;
}
let { value = getValue() } = $props();
</script>
```
There is another example that causes the `state_unsafe_mutation` and how
to fix (this happened on the tests that i changed):
```html
<script>
let log = $state([]);
function fallbackExample => {
log.push('fallback called');
return 1; // any value, just to show the issue with the log
}
let { value = fallbackExample() } = $props();
</script>
```
Here, we can see that `log` variable is a state. Before this PR, as i
said, this function `fallbackExample` will be executed once. So the logs
will be computed when the component is mounted. So there is no reason to
make the `log` a state. The simplest way to fix this is to make it a
normal variable:
```html
<script>
let log = [];
</script>
```
But with this PR, the function might be recalculated at some point, and
the `log` with a state makes sense now, so how to migrate in this case?
As i said, changing a state inside a lazy prop function is not a good
practice, we can think in a way to invert this dependency, and change
the approach from push (imperative mutation) to pull (declarative
derivation).
If a developer really needs to track how many times a fallback is
executed or react to its changes, they should use a $derived or an
$effect that observes the same dependencies as the fallback, or simply
observe the property itself:
```html
<script>
let { value = fallbackExample() } = $props();
let log = $state([]);
$effect(() => {
const message = `${value}`;
untrack(() => { // we don't want to track changes on the log variable
log.push(message);
});
});
</script>
```
### After all, how to migrate?
1. **If there is no state mutation inside the prop function, no need to
changes**;
2. **If there is a state mutation inside the prop function, but the
value muted is declared inside the same component:** remove the state
from it. Before this PR the method will be executed only once, and to
get the same result, you do not need the variable to be a state;
**Before**
```html
<script>
let log = $state([]);
const fallback_fn = () => {
log.push('fallback_fn');
return 1;
}
const { myProp = fallback_fn() } = $props();
</script>
```
**After**
```html
<script>
let log = [];
const fallback_fn = () => {
log.push('fallback_fn');
return 1;
}
const { myProp = fallback_fn() } = $props();
</script>
```
3. **If there is a state mutation inside the prop function, and the
value is read in multiple places:** change your approach, use a effect
to detect the change on the prop, and apply your mutation inside the
effect;
Before:
```html
<script>
import { setLog } from './logs.js'; // setLog apply a mutation on a state
const fallback_fn = () => {
setLog('fallback_fn');
return 1;
}
const { myProp = fallback_fn() } = $props();
</script>
```
After
```html
<script>
import { setLog } from './logs.js'; // setLog apply a mutation on a state
const fallback_fn = () => {
return 1;
}
const { myProp = fallback_fn() } = $props();
$effect(() => {
const message = `${myProp}`;
untrack(() => {
setLog(message);
});
});
</script>
```
### Severity (number of people affected x effort): Low
- **Affected Users:** Minimal. Mutating state inside a property
initializer is a rare edge case and considered an anti-pattern (because
its a side effect inside a getter). Most users use constants or pure
functions for fallbacks.
- **Migration Effort:** Low. As demonstrated in the examples above, the
fix usually involves either removing an unnecessary $state or moving the
side effect to its proper place, the $effect
### Conclusion
This PR encourages users to program in a better way. Forcing a clean
separation between data and their side effects. The developer can use
this new feature mainly in i18n services, providing better usability and
experience. Also, this PR makes the properties more predictable, since
the expected behavior is that it works reactively, eliminating this bug
for future developers.
Even though this PR adds a breaking change, it's easily solvable, and
the chance of any user facing this problem is low.
**Full example to test reactivity in props** (won't work on web, you can
get the PR and test localy to see it working):
https://svelte.dev/playground/a6608434d8c642179f0e2b72468c74d7?version=latest
*A unit test for this reactivity was created:
runtime-runes/props-default-value-reactivity*.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
The logic of checking that the current batch is still the generated one
is flawed. If microtasks align the current batch can be a different
value even if the batch still need to be flushed. This therefore
switches the heuristic to what it actually should express: "has this
batch run already?"
Fixes#18126 (because the batch isn't running, it later runs into the
invariant)
Fixes#18134.
## Problem
`read_value()` in
`packages/svelte/src/compiler/phases/1-parse/read/style.js` has no logic
to skip CSS block comments (`/* ... */`). When the parser encounters an
apostrophe inside a comment, it sets `quote_mark = "'"` — treating it as
the start of a string literal — then never finds a matching closing
quote, and ultimately throws `unexpected_eof` at the end of the style
block.
Minimal repro:
```svelte
<style>
/* it's a comment */
.foo { color: red; }
</style>
```
→ `Error: Unexpected end of input`
## Fix
Add a `/* ... */` skip path inside the `read_value` loop, mirroring the
same pattern already used in `allow_comment_or_whitespace`. When `/*` is
detected outside a string or url context, the parser advances past the
entire comment without adding its content to the value string.
---------
Co-authored-by: Dor Alagem <doralagem@MacBook-Pro-sl-Dor.local>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Fixes#18145
I wonder why nulling happens after updating `bind:this` but not before,
which would fix the issue as well, though not as efficiently.
### 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`
### 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.
- [ ] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).
### Tests and linting
- [ ] Run the tests with `pnpm test` and lint the project with `pnpm
lint`
`$state.snapshot` already clones the value returned from `toJSON`, and
its `Snapshot<T>` type reflects that return type. The `$state.snapshot`
docs now call out that behavior explicitly, including the generated
ambient types shown by editors.
Test plan: `pnpm check`; `pnpm lint`.
Fixes#18129
Co-authored-by: sakaenyeniceri5 <sakaenyeniceri5@users.noreply.github.com>
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.
# Releases
## svelte@5.55.5
### Patch Changes
- fix: don't mark deriveds while an effect is updating
([#18124](https://github.com/sveltejs/svelte/pull/18124))
- fix: do not dispatch introstart event with animation of animate
directive ([#18122](https://github.com/sveltejs/svelte/pull/18122))
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes#18123
This makes setting state inside effects slightly slower theoretically
(since they hit the new guard), but I verified that the original issue
for which we introduced this (#16658) is still fast with this change.
The more we add logic to this the more I think we should investigate
switching to a different mechanism. I tried using a `Set` previously but
it did hurt the benchmarks a bit - might try to revisit a variant of
this.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
closes#18056
related: #17567 and #14009
# Changes
move `dispatch_event()` calls in `transitions.js` out of `animate()`
function using an additional `on_begin()` callback parameter. Doing so
makes it possible to dispatch the `introstart` and `outrostart` events
only from `transition()`.
# Testing
add a test checking that svelte dispatches no event when it runs an
animation
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.
# Releases
## svelte@5.55.4
### Patch Changes
- fix: never mark a child effect root as inert
([#18111](https://github.com/sveltejs/svelte/pull/18111))
- fix: reset context after waiting on blockers of `@const` expressions
([#18100](https://github.com/sveltejs/svelte/pull/18100))
- fix: keep flushing new eager effects
([#18102](https://github.com/sveltejs/svelte/pull/18102))
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
A nested `$effect.root` was marked `INERT` during `pause_children`,
which caused it to stay in that state indefinetly after the rest of the
parent tree was destroyed. Consequently deriveds inside no longer update
and cause warnings.
This fixes it by not marking nested `$effect.root`s as inert, just like
nested `$effect.root`s are not destryoed and instead become a new root.
Fixes#18097
Regression from #18039 - we need to have each await expression (and
waiting on blockers is one) in its own entry of `(renderer.)run`. Else
context is not restored correctly and if the synchronous expression
afterwards requires it stuff breaks.
Fixes#18098
The previous code "swallowed" new additions to the array of eager
effects that happened while flushing since `eager_flush` did not clear
the array before running, only afterwards. Now it clears the beforehand,
causing newly added eager effects to run, too.
An example where this can happen is `$state.eager` and `$effect.pending`
in combination: first `$state.eager` is flushed, then due to `flushSync`
the `queue_micro_task` inside `boundary.js` that flushes
`$effect.pending` is triggered synchronously, adding new entries to the
`eager_versions` array. If they're only cleared at the end of
`eager_flush`, new entries are swallowed.
Related to #18095 (but not fixing it yet)
Avoids two categories of false positives for reactivity loss warning:
1. if you have synchronously read signals already as part of invoking
the async_derived function, then it shouldn't warn when these signals
are read after an await if we know they haven't changed (we check the
write version for that)
2. `track_reactivity_loss` kept the `reactivity_loss_tracker` around
indefinitely, both when invoking the async operation as well as when
it's finished. The former is buggy because while the async operation
happens unrelated reads as part of other reactivity work can happen, the
latter is buggy because if it's the last in a chain of awaits it's kept
around until the next async work starts.
Fixes https://github.com/sveltejs/kit/issues/15654
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Per
https://github.com/sveltejs/svelte/pull/17862#issuecomment-4049752548,
this freezes the value of a derived if it was created inside a parent
effect that is now destroyed. This prevents the sort of bug where a
derived reads `foo.bar` even though `foo` is now `undefined`.
If the derived is dirty, a warning will be printed.
This PR also gets rid of some weirdness around `derived.parent` — it can
only ever be an `Effect | null`, and there's no need for
`get_derived_parent_effect`.
Blocked on https://github.com/sveltejs/kit/pull/15533 and a follow-up
that switches remote functions to use `$effect.root` (since this
effectively undoes #17171), hence draft.
### 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: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Move the calculation of blockers into the analysis phase and then only
push the right thunks in the transform phase - similar to how we already
do it with top level `$.run`
Fixes#18024
The solution is a tiny bit brittle (not much more than the top level one
we already have) and I tried to make it a bit more robust but ended up
in a rabbit hole in #18032 - we can revisit that solution once all the
old stuff is gone. Until then this is the most pragmatic/non-invasive
change.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
The BranchManager in `branches.js` does create a temporary anchor when
creating a new branch offscreen, and deletes it once the branch is
committed. Normally this is fine, but the combination of HMR and dynamic
components leads to a bug: Since `svelte-component.js` passes the
temporary anchor along to the component it generates, which is the HMR
wrapper, this wrapper will have an obsolete, disconnected anchor on
updates, leading to the content disappearing.
The fix is to add a dev-only symbol which we set on the original (then
obsolete) anchor to tell about the updated anchor that should be used
for HMR updates.
Fixes https://github.com/sveltejs/kit/issues/14699Fixes#17211
RIght now, when an async error occurs inside a fork, the error UI will
show immediately. This change defers the removal of the current content
etc until the fork is committed. The BranchManager logic cannot be
reused here since this is not about pending work, and we cannot wait for
other pending work to complete if the fork commits. Instead, batches now
have a "on fork commit" callback set which the boundary pushes to. The
boundary's effects are skipped in the fork until it's committing, at
which point we "resume" the error logic (calling onerror, transforming
it, etc)
Fixes#18060
small tidy-up spurred by #17954 — rather than try-catching every
`parse_expression_at` call and passing the error to
`parser.acorn_error`, we can handle the error locally and get rid of
that method
### Before submitting the PR, please make sure you do the following
- [ ] 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.
- [ ] 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: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
While looking into #17954 I realised that a) our code for handling
parentheses in expressions is unnecessarily convoluted and b) it doesn't
handle the case where you have an opening parent outside the first
comment — this fails to parse:
```svelte
{(/**/ 42)}
```
This fixes it and simplifies the code a good bit.
### 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`
Fixes#17571 where the situation is the following:
A derived creates a new query. That query initializes loading with true.
This means the if block is marked for destruction (therefore effects
inside branch are skipped), but it's not doing that yet because the
query promise is pending. Then query resolves and loading is set back to
false right before resolving, but it's not the same tick so
`loading=false` is a separate thing. Because that later batch doesn't
see any overlap with an earlier batch (the earlier batch did set loading
to true but not via set but indirectly via recreating the query) it
doesn't wait on it and flushes right away. Now the if block is marked as
visible again but the earlier batch doesn't know that if noone unskips
its branch. If we don't do that the render effect that is now dirty as
part of that batch will not run.
Fixes part of #17940 (the hydration->error thing still needs a repro).
Essentially in sync mode render effects are executed during traversing
the effect tree, and when flushSync is called during that it can cause
the timing of things getting out of sync such that you end up wanting to
rebase another branch, which is never needed in sync mode.
So we just skip that logic in sync mode. Another way would be to adjust
flushSync such that it does appends itself to the end of the current
flushing cycle if it notices processing is already ongoing. This will
not work when you pass a callback function to `flushSync` though -
another indicator that we should probably remove that callback argument.
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
If a new branch is created (e.g. if block becomes truthy) inside a fork
and a derived is read inside the new branch for the first time, it will
get reactions but its true value will stay uninitialized due to the
fork. If the fork then commits, it will NOT change its value because it
will not reexecute: there is no effect running after commit telling it
to do that, because it was only read inside new effects which already
ran while still inside the fork.
Now, when that branch is removed (e.g. if block becomes falsy), the
derived loses its reactions again and becomes disconnected, at which
point the status is reset. So we end up with a maybe_dirty or clean
derived and an uninitialized value, causing breakage if it's called
again afterwards.
The fix is to not reset the status in this case.
Fixes https://github.com/sveltejs/kit/issues/15126
Fixes https://github.com/sveltejs/kit/issues/15318
Fixes https://github.com/sveltejs/kit/issues/15061
`<details>` elements fire `ontoggle` as ToggleEvents
([source](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/toggle_event)),
but they're currently just typed as Event.
### Before submitting the PR, please make sure you do the following
- [ ] 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:`.
- [ ] 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`
Gets rid of some annoying console spam. I don't totally understand why
`warnings` isn't populated by the time the assertions happen (it seems
to happen just afterwards, except if I sleep for a few milliseconds and
then do the assertion the warnings get swallowed altogether?) but for
right now I don't much care
This started out as me implementing
https://github.com/sveltejs/svelte/pull/17998/changes#r3018047965, but
then I realised that I'd also fixed the bug that #17998 addresses. So I
guess it's an alternative to that PR
---------
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
## Description
**While investigating the compiler analysis phase for errors or missing
lines, I located an explicit `TODO fix the message here` comment in the
source code
(`packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js`).**
### Triggering Code
```svelte
<script module>
import { foo } from './somewhere.js';
</script>
<script>
let foo = 'conflict'; // Triggers the duplicate module import error
</script>
```
## The Bug
When a component author creates a local `let` declaration in the
instance `<script>` block that shadows a variable imported in the
`<script module>` block, the compiler emits a
`declaration_duplicate_module_import` error.
However, the error message text was misleading. It stated:
```diff
- Cannot declare a variable with the same name as an import inside `<script module>`
+ Cannot declare a variable with the same name as an import from `<script module>`
```
This incorrectly implies that the duplicate declaration itself is
happening *inside* the module script block, rather than colliding with
an import *from* that block.
## Changes Made
| Action | Target (File / Function) | Description |
| :--- | :--- | :--- |
| **Template Fix** | `packages/svelte/messages/compile-errors/script.md`
| Changed "inside" to "from" to accurately reflect the nature of the
shadow collision. |
| **Regenerated Errors** | `packages/svelte/src/compiler/errors.js` |
Ran `pnpm generate` to apply the updated markdown template into the
dictionary list. |
| **Removed TODO** | `ensure_no_module_import_conflict` function |
Removed the original `// TODO fix the message here` notation from the
validator. |
| **Test Adjustments** | `illegal-variable-declaration/errors.json` |
Updated the validator test suite snapshots to assert against the correct
error message format. |
## Validation Results
* The error correctly throws when a user replicates this component
structure.
* Vitest suite `tests/validator/test.ts` successfully asserts the new,
clearer message.
---
### 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`
# THANK YOU
Closes#18012
We were removing `superTypeArguments` but the AST showed
`superTypeParameters`.
Luckily, I was able to pinpoint `esrap@2.2.4` as the cause... I've only
bumped `esrap` and that made the test fail (so that I could fix it)
And of course it was that...there's literally a commit that explicitly
print them lol
f9137c4101
If a batch creates a new branch (e.g. through an if block becoming true)
the previous batches so far do not know about the new effects created
through that. This can lead to stale values being shown. We therefore
schedule those new effects on prior batches if they are touched by a
`current` value of that batch
Fixes#17099
extracted from #17971
Simon H
Rich Harris
Elliott Johnson
RonGamzu
Mathias Picker
Karl Georgsen
wheelman
Puneet Dixit
github-actions[bot]
Sean Kenneth Doherty
OfirHaf
quyentonndbs
Paolo Ricciuti
Rohan Santhosh Kumar
Eduardo Kurek
Dor Alagem
7nik
Wesley
dabund24
ottomated
Rohit Nair P