Merge branch 'main' into svelte-custom-renderer

svelte-custom-renderer
Paolo Ricciuti 3 weeks ago committed by GitHub
commit 20bd0cfc13
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: correctly calculate `@const` blockers

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: freeze deriveds once their containing effects are destroyed

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: avoid false positives for reactivity loss warning

@ -134,6 +134,14 @@ When logging a [proxy](https://developer.mozilla.org/en-US/docs/Web/JavaScript/R
The easiest way to log a value as it changes over time is to use the [`$inspect`](/docs/svelte/$inspect) rune. Alternatively, to log things on a one-off basis (for example, inside an event handler) you can use [`$state.snapshot`](/docs/svelte/$state#$state.snapshot) to take a snapshot of the current value.
### derived_inert
```
Reading a derived belonging to a now-destroyed effect may result in stale values
```
A `$derived` value created inside an effect will stop updating when the effect is destroyed. You should create the `$derived` outside the effect, or inside an `$effect.root`.
### event_handler_invalid
```

@ -120,6 +120,12 @@ When logging a [proxy](https://developer.mozilla.org/en-US/docs/Web/JavaScript/R
The easiest way to log a value as it changes over time is to use the [`$inspect`](/docs/svelte/$inspect) rune. Alternatively, to log things on a one-off basis (for example, inside an event handler) you can use [`$state.snapshot`](/docs/svelte/$state#$state.snapshot) to take a snapshot of the current value.
## derived_inert
> Reading a derived belonging to a now-destroyed effect may result in stale values
A `$derived` value created inside an effect will stop updating when the effect is destroyed. You should create the `$derived` outside the effect, or inside an `$effect.root`.
## event_handler_invalid
> %handler% should be a function. Did you mean to %suggestion%?

@ -2,6 +2,7 @@ import type { Scope } from '../scope.js';
import type { ComponentAnalysis, ReactiveStatement } from '../types.js';
import type { AST, StateField, ValidatedCompileOptions } from '#compiler';
import type { ExpressionMetadata } from '../nodes.js';
import type { Identifier } from 'estree';
export interface AnalysisState {
scope: Scope;
@ -33,6 +34,13 @@ export interface AnalysisState {
* Set when we're inside a `$derived(...)` expression (but not `$derived.by(...)`) or `@const`
*/
derived_function_depth: number;
/** Collected info about async `{@const }` declarations */
async_consts?: {
id: Identifier;
/** How many `@const` declarations there are (already) in this scope */
declaration_count: number;
};
}
export type Context<State extends AnalysisState = AnalysisState> = import('zimmerframe').Context<

@ -1,6 +1,7 @@
/** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */
import * as e from '../../../errors.js';
import * as b from '#compiler/builders';
import { validate_opening_tag } from './shared/utils.js';
/**
@ -42,4 +43,28 @@ export function ConstTag(node, context) {
function_depth: context.state.function_depth + 1,
derived_function_depth: context.state.function_depth + 1
});
const has_await = node.metadata.expression.has_await;
const blockers = [...node.metadata.expression.dependencies]
.map((dep) => dep.blocker)
.filter((b) => b !== null && b.object !== context.state.async_consts?.id);
if (has_await || context.state.async_consts || blockers.length > 0) {
const run = (context.state.async_consts ??= {
id: context.state.analysis.root.unique('promises'),
declaration_count: 0
});
node.metadata.promises_id = run.id;
const bindings = context.state.scope.get_bindings(declaration);
// keep the counter in sync with the number of thunks pushed in ConstTag in transform
// TODO 6.0 once non-async and non-runes mode is gone investigate making this more robust
// via something like the approach in https://github.com/sveltejs/svelte/pull/18032
const length = run.declaration_count++;
const blocker = b.member(run.id, b.literal(length), true);
for (const binding of bindings) {
binding.blocker = blocker;
}
}
}

@ -6,5 +6,5 @@
* @param {Context} context
*/
export function Fragment(node, context) {
context.next({ ...context.state, fragment: node });
context.next({ ...context.state, fragment: node, async_consts: undefined });
}

@ -1,7 +1,6 @@
/** @import { Expression, Identifier, Pattern } from 'estree' */
/** @import { Expression, Identifier, Pattern, Statement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
/** @import { ExpressionMetadata } from '../../../nodes.js' */
import { dev } from '../../../../state.js';
import { extract_identifiers } from '../../../../utils/ast.js';
import * as b from '#compiler/builders';
@ -27,13 +26,7 @@ export function ConstTag(node, context) {
context.state.transform[declaration.id.name] = { read: get_value };
add_const_declaration(
context.state,
declaration.id,
expression,
node.metadata.expression,
context.state.scope.get_bindings(declaration)
);
add_const_declaration(context.state, declaration.id, expression, node.metadata);
} else {
const identifiers = extract_identifiers(declaration.id);
const tmp = b.id(context.state.scope.generate('computed_const'));
@ -70,13 +63,7 @@ export function ConstTag(node, context) {
expression = b.call('$.tag', expression, b.literal('[@const]'));
}
add_const_declaration(
context.state,
tmp,
expression,
node.metadata.expression,
context.state.scope.get_bindings(declaration)
);
add_const_declaration(context.state, tmp, expression, node.metadata);
for (const node of identifiers) {
context.state.transform[node.name] = {
@ -90,42 +77,40 @@ export function ConstTag(node, context) {
* @param {ComponentContext['state']} state
* @param {Identifier} id
* @param {Expression} expression
* @param {ExpressionMetadata} metadata
* @param {import('#compiler').Binding[]} bindings
* @param {AST.ConstTag['metadata']} metadata
*/
function add_const_declaration(state, id, expression, metadata, bindings) {
function add_const_declaration(state, id, expression, metadata) {
// we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors
const after = dev ? [b.stmt(b.call('$.get', id))] : [];
const has_await = metadata.has_await;
const blockers = [...metadata.dependencies]
const blockers = [...metadata.expression.dependencies]
.map((dep) => dep.blocker)
.filter((b) => b !== null && b.object !== state.async_consts?.id);
if (has_await || state.async_consts || blockers.length > 0) {
if (metadata.promises_id) {
const run = (state.async_consts ??= {
id: b.id(state.scope.generate('promises')),
id: metadata.promises_id,
thunks: []
});
state.consts.push(b.let(id));
const assignment = b.assignment('=', id, expression);
const body = after.length === 0 ? assignment : b.block([b.stmt(assignment), ...after]);
/** @type {Statement | undefined} */
let promise_stmt;
if (blockers.length === 1) {
run.thunks.push(b.thunk(b.member(/** @type {Expression} */ (blockers[0]), 'promise')));
promise_stmt = b.stmt(b.await(b.member(/** @type {Expression} */ (blockers[0]), 'promise')));
} else if (blockers.length > 0) {
run.thunks.push(b.thunk(b.call('$.wait', b.array(blockers))));
promise_stmt = b.stmt(b.await(b.call('$.wait', b.array(blockers))));
}
run.thunks.push(b.thunk(body, has_await));
const blocker = b.member(run.id, b.literal(run.thunks.length - 1), true);
for (const binding of bindings) {
binding.blocker = blocker;
// keep the number of thunks pushed in sync with ConstTag in analysis phase
const assignment = b.assignment('=', id, expression);
if (promise_stmt) {
run.thunks.push(b.thunk(b.block([promise_stmt, b.stmt(assignment)]), true));
} else {
run.thunks.push(b.thunk(assignment, metadata.expression.has_await));
}
} else {
state.consts.push(b.const(id, expression));

@ -1,4 +1,4 @@
/** @import { Expression, Pattern } from 'estree' */
/** @import { Expression, Pattern, Statement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types.js' */
import * as b from '#compiler/builders';
@ -12,36 +12,37 @@ export function ConstTag(node, context) {
const declaration = node.declaration.declarations[0];
const id = /** @type {Pattern} */ (context.visit(declaration.id));
const init = /** @type {Expression} */ (context.visit(declaration.init));
const has_await = node.metadata.expression.has_await;
const blockers = [...node.metadata.expression.dependencies]
.map((dep) => dep.blocker)
.filter((b) => b !== null && b.object !== context.state.async_consts?.id);
if (has_await || context.state.async_consts || blockers.length > 0) {
if (node.metadata.promises_id) {
const run = (context.state.async_consts ??= {
id: b.id(context.state.scope.generate('promises')),
id: node.metadata.promises_id,
thunks: []
});
const identifiers = extract_identifiers(declaration.id);
const bindings = context.state.scope.get_bindings(declaration);
for (const identifier of identifiers) {
context.state.init.push(b.let(identifier.name));
}
/** @type {Statement | undefined} */
let promise_stmt;
if (blockers.length === 1) {
run.thunks.push(b.thunk(/** @type {Expression} */ (blockers[0])));
promise_stmt = b.stmt(b.await(/** @type {Expression} */ (blockers[0])));
} else if (blockers.length > 0) {
run.thunks.push(b.thunk(b.call('Promise.all', b.array(blockers))));
promise_stmt = b.stmt(b.await(b.call('Promise.all', b.array(blockers))));
}
// keep the number of thunks pushed in sync with ConstTag in analysis phase
const assignment = b.assignment('=', id, init);
run.thunks.push(b.thunk(b.block([b.stmt(assignment)]), has_await));
const blocker = b.member(run.id, b.literal(run.thunks.length - 1), true);
for (const binding of bindings) {
binding.blocker = blocker;
if (promise_stmt) {
run.thunks.push(b.thunk(b.block([promise_stmt, b.stmt(assignment)]), true));
} else {
run.thunks.push(b.thunk(assignment, node.metadata.expression.has_await));
}
} else {
context.state.init.push(b.const(id, init));

@ -156,6 +156,8 @@ export namespace AST {
/** @internal */
metadata: {
expression: ExpressionMetadata;
/** If this const tag contains an await expression, or needs to wait on other async, this is set */
promises_id?: Identifier;
};
}

@ -168,10 +168,26 @@ export async function save(promise) {
*/
export async function track_reactivity_loss(promise) {
var previous_async_effect = reactivity_loss_tracker;
// Ensure that unrelated reads after an async operation is kicked off don't cause false positives
queueMicrotask(() => {
if (reactivity_loss_tracker === previous_async_effect) {
set_reactivity_loss_tracker(null);
}
});
var value = await promise;
return () => {
set_reactivity_loss_tracker(previous_async_effect);
// While this can result in false negatives it also guards against the more important
// false positives that would occur if this is the last in a chain of async operations,
// and the reactivity_loss_tracker would then stay around until the next async operation happens.
queueMicrotask(() => {
if (reactivity_loss_tracker === previous_async_effect) {
set_reactivity_loss_tracker(null);
}
});
return value;
};
}
@ -210,7 +226,9 @@ export async function* for_await_track_reactivity_loss(iterable) {
normal_completion = true;
break;
}
var prev = reactivity_loss_tracker;
yield value;
set_reactivity_loss_tracker(prev);
}
} finally {
// If the iterator had an abrupt completion and `return` is defined on the iterator, call it and return the value

@ -1,4 +1,4 @@
/** @import { Derived, Effect, Source } from '#client' */
/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */
/** @import { Batch } from './batch.js'; */
/** @import { Boundary } from '../dom/blocks/boundary.js'; */
import { DEV } from 'esm-env';
@ -12,7 +12,8 @@ import {
WAS_MARKED,
DESTROYED,
CLEAN,
REACTION_RAN
REACTION_RAN,
INERT
} from '#client/constants';
import {
active_reaction,
@ -23,7 +24,9 @@ import {
push_reaction_value,
is_destroying_effect,
update_effect,
remove_reactions
remove_reactions,
skipped_deps,
new_deps
} from '../runtime.js';
import { equals, safe_equals } from './equality.js';
import * as e from '../errors.js';
@ -48,11 +51,11 @@ import { set_signal_status, update_derived_status } from './status.js';
/**
* This allows us to track 'reactivity loss' that occurs when signals
* are read after a non-context-restoring `await`. Dev-only
* @type {{ effect: Effect, warned: boolean } | null}
* @type {{ effect: Effect, effect_deps: Set<Value>, warned: boolean } | null}
*/
export let reactivity_loss_tracker = null;
/** @param {{ effect: Effect, warned: boolean } | null} v */
/** @param {{ effect: Effect, effect_deps: Set<Value>, warned: boolean } | null} v */
export function set_reactivity_loss_tracker(v) {
reactivity_loss_tracker = v;
}
@ -67,10 +70,6 @@ export const recent_async_deriveds = new Set();
/*#__NO_SIDE_EFFECTS__*/
export function derived(fn) {
var flags = DERIVED | DIRTY;
var parent_derived =
active_reaction !== null && (active_reaction.f & DERIVED) !== 0
? /** @type {Derived} */ (active_reaction)
: null;
if (active_effect !== null) {
// Since deriveds are evaluated lazily, any effects created inside them are
@ -90,7 +89,7 @@ export function derived(fn) {
rv: 0,
v: /** @type {V} */ (UNINITIALIZED),
wv: 0,
parent: parent_derived ?? active_effect,
parent: active_effect,
ac: null
};
@ -128,15 +127,12 @@ export function async_derived(fn, label, location) {
var deferreds = new Map();
async_effect(() => {
var effect = /** @type {Effect} */ (active_effect);
if (DEV) {
reactivity_loss_tracker = {
effect: /** @type {Effect} */ (active_effect),
warned: false
};
reactivity_loss_tracker = { effect, effect_deps: new Set(), warned: false };
}
var effect = /** @type {Effect} */ (active_effect);
/** @type {ReturnType<typeof deferred<V>>} */
var d = deferred();
promise = d.promise;
@ -152,6 +148,24 @@ export function async_derived(fn, label, location) {
}
if (DEV) {
if (reactivity_loss_tracker) {
// Reused deps from previous run (indices 0 to skipped_deps-1)
// We deliberately only track direct dependencies of the async expression to encourage
// dependencies being directly visible at the point of the expression
if (effect.deps !== null) {
for (let i = 0; i < skipped_deps; i += 1) {
reactivity_loss_tracker.effect_deps.add(effect.deps[i]);
}
}
// New deps discovered this run
if (new_deps !== null) {
for (let i = 0; i < new_deps.length; i += 1) {
reactivity_loss_tracker.effect_deps.add(new_deps[i]);
}
}
}
reactivity_loss_tracker = null;
}
@ -320,23 +334,6 @@ export function destroy_derived_effects(derived) {
*/
let stack = [];
/**
* @param {Derived} derived
* @returns {Effect | null}
*/
function get_derived_parent_effect(derived) {
var parent = derived.parent;
while (parent !== null) {
if ((parent.f & DERIVED) === 0) {
// The original parent effect might've been destroyed but the derived
// is used elsewhere now - do not return the destroyed effect in that case
return (parent.f & DESTROYED) === 0 ? /** @type {Effect} */ (parent) : null;
}
parent = parent.parent;
}
return null;
}
/**
* @template T
* @param {Derived} derived
@ -345,8 +342,15 @@ function get_derived_parent_effect(derived) {
export function execute_derived(derived) {
var value;
var prev_active_effect = active_effect;
var parent = derived.parent;
if (!is_destroying_effect && parent !== null && (parent.f & (DESTROYED | INERT)) !== 0) {
w.derived_inert();
return derived.v;
}
set_active_effect(get_derived_parent_effect(derived));
set_active_effect(parent);
if (DEV) {
let prev_eager_effects = eager_effects;

@ -58,8 +58,8 @@ export interface Derived<V = unknown> extends Value<V>, Reaction {
fn: () => V;
/** Effects created inside this signal. Used to destroy those effects when the derived reruns or is cleaned up */
effects: null | Effect[];
/** Parent effect or derived */
parent: Effect | Derived | null;
/** Parent effect */
parent: Effect | null;
}
export interface EffectNodes {

@ -112,9 +112,9 @@ export function push_reaction_value(value) {
* and until a new dependency is accessed we track this via `skipped_deps`
* @type {null | Value[]}
*/
let new_deps = null;
export let new_deps = null;
let skipped_deps = 0;
export let skipped_deps = 0;
/**
* Tracks writes that the effect it's executed in doesn't listen to yet,
@ -585,7 +585,8 @@ export function get(signal) {
!untracking &&
reactivity_loss_tracker &&
!reactivity_loss_tracker.warned &&
(reactivity_loss_tracker.effect.f & REACTION_IS_UPDATING) === 0
(reactivity_loss_tracker.effect.f & REACTION_IS_UPDATING) === 0 &&
!reactivity_loss_tracker.effect_deps.has(signal)
) {
reactivity_loss_tracker.warned = true;

@ -74,6 +74,17 @@ export function console_log_state(method) {
}
}
/**
* Reading a derived belonging to a now-destroyed effect may result in stale values
*/
export function derived_inert() {
if (DEV) {
console.warn(`%c[svelte] derived_inert\n%cReading a derived belonging to a now-destroyed effect may result in stale values\nhttps://svelte.dev/e/derived_inert`, bold, normal);
} else {
console.warn(`https://svelte.dev/e/derived_inert`);
}
}
/**
* %handler% should be a function. Did you mean to %suggestion%?
* @param {string} handler

@ -0,0 +1,9 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await tick();
assert.htmlEqual(target.innerHTML, '<p>data</p>');
}
});

@ -0,0 +1,16 @@
<script>
let d = $derived(await Promise.resolve({ data: "data", hasData: true }));
let showFetchCta = $derived(d.hasData)
</script>
{#if d}
{@const {data, hasData} = d}
{#if hasData}
<p>{data}</p>
{:else if showFetchCta}
<p>Fetch now</p>
{:else}
<p>No data</p>
{/if}
{/if}

@ -0,0 +1,27 @@
import { tick } from 'svelte';
import { test } from '../../test';
import { normalise_trace_logs } from '../../../helpers.js';
export default test({
compileOptions: {
dev: true
},
async test({ assert, target, warnings }) {
await new Promise((r) => setTimeout(r, 25));
const [count] = target.querySelectorAll('button');
count.click();
await new Promise((r) => setTimeout(r, 25));
assert.deepEqual(normalise_trace_logs(warnings), [
{
log: 'Detected reactivity loss when reading `other`. This happens when state is read in an async function after an earlier `await`'
},
{
log: 'Detected reactivity loss when reading `other`. This happens when state is read in an async function after an earlier `await`'
}
]);
}
});

@ -0,0 +1,27 @@
<script>
let count = $state(0);
let other = $state(0);
function delayed(value, ms = 1000) {
return new Promise((f) => setTimeout(() => f(value), ms))
}
async function foo() {
await new Promise(r => setTimeout(r, 10));
}
async function bar() {
const value = await delayed(count, 10);
other; // should trigger warning
return value;
}
async function get() {
foo();
return await bar();
}
</script>
<button onclick={() => count++}>{count}</button>
<button onclick={() => other++}>{other}</button>
{await get()}

@ -14,7 +14,7 @@ export default test({
assert.deepEqual(normalise_trace_logs(warnings), [
{
log: 'Detected reactivity loss when reading `values.length`. This happens when state is read in an async function after an earlier `await`'
log: 'Detected reactivity loss when reading `values[1]`. This happens when state is read in an async function after an earlier `await`'
}
]);
}

@ -15,7 +15,7 @@ export default test({
assert.deepEqual(normalise_trace_logs(warnings), [
{
log: 'Detected reactivity loss when reading `values.length`. This happens when state is read in an async function after an earlier `await`'
log: 'Detected reactivity loss when reading `values[1]`. This happens when state is read in an async function after an earlier `await`'
}
]);
}

@ -0,0 +1,21 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true
},
async test({ assert, target, warnings }) {
await tick();
const [x, y] = target.querySelectorAll('button');
y.click();
await tick();
x.click();
await new Promise((r) => setTimeout(r, 15));
assert.equal(warnings.length, 0);
}
});

@ -0,0 +1,16 @@
<script>
let x = $state(0);
let y = $state(0);
async function foo(x) {
if (x) {
await 1; // restores reactivity loss warning context
await new Promise(r => setTimeout(r, 10)) // saves reactivity loss warning context; should not keep it while running
}
return x
}
</script>
{x} {await foo(y)}
<button onclick={() => x += 1}>x++</button>
<button onclick={() => y += 1}>y++</button>

@ -0,0 +1,19 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true
},
async test({ assert, target, warnings }) {
await tick();
const [count] = target.querySelectorAll('button');
count.click();
await tick();
assert.equal(warnings.length, 0);
}
});

@ -0,0 +1,14 @@
<script>
let count = $state(0);
async function delay(c) {
if (c) {
await new Promise(r => setTimeout(r));
count; // count already read synchronously; should not result in reacitive loss warning
}
return c;
}
</script>
{await delay(count)}
<button onclick={() => count += 1}>count++</button>

@ -0,0 +1,19 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true
},
async test({ assert, target, warnings }) {
await new Promise((r) => setTimeout(r, 5));
const [count] = target.querySelectorAll('button');
count.click();
await tick();
assert.equal(warnings.length, 0);
}
});

@ -0,0 +1,14 @@
<script>
let count = $state(0);
async function run() {
await new Promise(r => setTimeout(r));
}
async function get() {
run();
return 1;
}
</script>
<button onclick={() => count++}>{count}</button>
{await get()}

@ -1391,8 +1391,12 @@ describe('signals', () => {
};
});
test('derived whose original parent effect has been destroyed keeps updating', () => {
test('derived whose original parent effect has been destroyed no longer updates', () => {
return () => {
const warn = console.warn;
const warnings: string[] = [];
console.warn = (...args) => warnings.push(...args);
let count: Source<number>;
let double: Derived<number>;
const destroy = effect_root(() => {
@ -1404,17 +1408,23 @@ describe('signals', () => {
flushSync();
assert.equal($.get(double!), 0);
destroy();
flushSync();
set(count!, 1);
flushSync();
assert.equal($.get(double!), 2);
destroy();
assert.equal($.get(double!), 2);
assert.equal(warnings.length, 0); // value is unchanged, no warning yet
set(count!, 2);
flushSync();
assert.equal($.get(double!), 4);
assert.equal($.get(double!), 2);
assert.ok(warnings.some((str) => str.includes('derived_inert')));
console.warn = warn;
};
});

@ -7,16 +7,7 @@ export default function Async_const($$renderer) {
let a;
let b;
var promises = $$renderer.run([
async () => {
a = (await $.save(1))();
},
() => {
b = a + 1;
}
]);
var promises = $$renderer.run([async () => a = (await $.save(1))(), () => b = a + 1]);
$$renderer.push(`<p>`);
$$renderer.async([promises[1]], ($$renderer) => $$renderer.push(() => $.escape(b)));

@ -28,25 +28,15 @@ export default function Async_in_derived($$renderer, $$props) {
let no2;
var promises = $$renderer.run([
async () => {
yes1 = (await $.save(1))();
},
async () => {
yes2 = foo((await $.save(1))());
},
() => {
no1 = (async () => {
return await 1;
})();
},
() => {
no2 = (async () => {
return await 1;
})();
}
async () => yes1 = (await $.save(1))(),
async () => yes2 = foo((await $.save(1))()),
() => no1 = (async () => {
return await 1;
})(),
() => no2 = (async () => {
return await 1;
})()
]);
} else {
$$renderer.push('<!--[-1-->');

Loading…
Cancel
Save