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
## Summary
- Fix the missing space before `<option>` in the bind documentation.
## Related issue
- N/A
## Guideline alignment
- Read `CONTRIBUTING.md` and kept this to one focused docs-only file
change.
- No behavior, test, fixture, changeset, or generated-file changes.
## Test plan
- `git diff --check`
- Not run: docs-only spacing fix.
Co-authored-by: Codex <codex@openai.com>
If project that is downloaded is a project that can't be transfered into
the playground (because it relies on SvelteKit stuff), instead of
failing it now asks if you want to put it into another folder within the
playgrounds folder
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
Paolo Ricciuti
Rich Harris
Simon H
Rohan Santhosh Kumar
Eduardo Kurek
Dor Alagem
7nik
Wesley
github-actions[bot]
dabund24