fix: make beforeUpdate/afterUpdate behave more like Svelte 4 (#10408)

Closes #9420.

This PR creates an $effect.pre (before beforeUpdate and an $effect (for afterUpdate) and, inside those, listen for all locally declared signals plus reactive props. This does mean that we need to link the locally declared signals to the component context (the reverse of the current behaviour, wherein we link the component context to locally declared signals).
pull/10430/head
Rich Harris 2 years ago committed by GitHub
parent 983db98756
commit 623340a1de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: align `beforeUpdate`/`afterUpdate` behavior better with that in Svelte 4

@ -200,6 +200,8 @@ const runes = {
`${rune} can only be called with ${list(args, 'or')} ${
args.length === 1 && args[0] === 1 ? 'argument' : 'arguments'
}`,
/** @param {string} name */
'invalid-runes-mode-import': (name) => `${name} cannot be used in runes mode`,
'duplicate-props-rune': () => `Cannot use $props() more than once`
};

@ -492,6 +492,20 @@ const validation = {
error(node, 'invalid-const-placement');
}
},
ImportDeclaration(node, context) {
if (node.source.value === 'svelte' && context.state.analysis.runes) {
for (const specifier of node.specifiers) {
if (specifier.type === 'ImportSpecifier') {
if (
specifier.imported.name === 'beforeUpdate' ||
specifier.imported.name === 'afterUpdate'
) {
error(specifier, 'invalid-runes-mode-import', specifier.imported.name);
}
}
}
}
},
LetDirective(node, context) {
const parent = context.path.at(-1);
if (

@ -265,6 +265,7 @@ export function client_component(source, analysis, options) {
...legacy_reactive_declarations,
...group_binding_declarations,
.../** @type {import('estree').Statement[]} */ (instance.body),
analysis.runes ? b.empty : b.stmt(b.call('$.init')),
.../** @type {import('estree').Statement[]} */ (template.body),
...static_bindings
]);

@ -1,6 +1,6 @@
import { DEV } from 'esm-env';
import { subscribe_to_store } from '../../store/utils.js';
import { noop, run_all } from '../common.js';
import { noop, run, run_all } from '../common.js';
import {
array_prototype,
get_descriptor,
@ -104,18 +104,9 @@ export let current_block = null;
/** @type {import('./types.js').ComponentContext | null} */
export let current_component_context = null;
export let is_ssr = false;
export let updating_derived = false;
/**
* @param {boolean} ssr
* @returns {void}
*/
export function set_is_ssr(ssr) {
is_ssr = ssr;
}
/**
* @param {null | import('./types.js').ComponentContext} context
* @returns {boolean}
@ -147,14 +138,6 @@ export function batch_inspect(target, prop, receiver) {
};
}
/**
* @param {null | import('./types.js').ComponentContext} context_stack_item
* @returns {void}
*/
export function set_current_component_context(context_stack_item) {
current_component_context = context_stack_item;
}
/**
* @param {unknown} a
* @param {unknown} b
@ -181,8 +164,6 @@ function create_source_signal(flags, value) {
f: flags,
// value
v: value,
// context: We can remove this if we get rid of beforeUpdate/afterUpdate
x: null,
// this is for DEV only
inspect: new Set()
};
@ -195,9 +176,7 @@ function create_source_signal(flags, value) {
// flags
f: flags,
// value
v: value,
// context: We can remove this if we get rid of beforeUpdate/afterUpdate
x: null
v: value
};
}
@ -346,12 +325,6 @@ function execute_signal_fn(signal) {
current_skip_consumer = !is_flushing_effect && (flags & UNOWNED) !== 0;
current_untracking = false;
// Render effects are invoked when the UI is about to be updated - run beforeUpdate at that point
if (is_render_effect && current_component_context?.u != null) {
// update_callbacks.execute()
current_component_context.u.e();
}
try {
let res;
if (is_render_effect) {
@ -924,7 +897,7 @@ export function store_set(store, value) {
* @param {import('./types.js').StoreReferencesContainer} stores
*/
export function unsubscribe_on_destroy(stores) {
onDestroy(() => {
on_destroy(() => {
let store_name;
for (store_name in stores) {
const ref = stores[store_name];
@ -1150,7 +1123,7 @@ export function mark_subtree_inert(signal, inert, visited_blocks = new Set()) {
* @returns {void}
*/
function mark_signal_consumers(signal, to_status, force_schedule) {
const runes = is_runes(signal.x);
const runes = is_runes(null);
const consumers = signal.c;
if (consumers !== null) {
const length = consumers.length;
@ -1192,7 +1165,7 @@ export function set_signal_value(signal, value) {
!current_untracking &&
!ignore_mutation_validation &&
current_consumer !== null &&
is_runes(signal.x) &&
is_runes(null) &&
(current_consumer.f & DERIVED) !== 0
) {
throw new Error(
@ -1208,7 +1181,6 @@ export function set_signal_value(signal, value) {
(signal.f & SOURCE) !== 0 &&
!(/** @type {import('./types.js').EqualsFunctions} */ (signal.e)(value, signal.v))
) {
const component_context = signal.x;
signal.v = value;
// If the current signal is running for the first time, it won't have any
// consumers as we only allocate and assign the consumers after the signal
@ -1219,7 +1191,7 @@ export function set_signal_value(signal, value) {
// $effect(() => x++)
//
if (
is_runes(component_context) &&
is_runes(null) &&
current_effect !== null &&
current_effect.c === null &&
(current_effect.f & CLEAN) !== 0
@ -1236,19 +1208,6 @@ export function set_signal_value(signal, value) {
}
}
mark_signal_consumers(signal, DIRTY, true);
// If we have afterUpdates locally on the component, but we're within a render effect
// then we will need to manually invoke the beforeUpdate/afterUpdate logic.
// TODO: should we put this being a is_runes check and only run it in non-runes mode?
if (current_effect === null && current_queued_pre_and_render_effects.length === 0) {
const update_callbacks = component_context?.u;
if (update_callbacks != null) {
run_all(update_callbacks.b);
const managed = managed_effect(() => {
destroy_signal(managed);
run_all(update_callbacks.a);
});
}
}
// @ts-expect-error
if (DEV && signal.inspect) {
@ -1299,7 +1258,7 @@ export function derived(init) {
create_computation_signal(flags | CLEAN, UNINITIALIZED, current_block)
);
signal.i = init;
signal.x = current_component_context;
bind_signal_to_component_context(signal);
signal.e = default_equals;
if (current_consumer !== null) {
push_reference(current_consumer, signal);
@ -1327,10 +1286,27 @@ export function derived_safe_equal(init) {
/*#__NO_SIDE_EFFECTS__*/
export function source(initial_value) {
const source = create_source_signal(SOURCE | CLEAN, initial_value);
source.x = current_component_context;
bind_signal_to_component_context(source);
return source;
}
/**
* This function binds a signal to the component context, so that we can fire
* beforeUpdate/afterUpdate callbacks at the correct time etc
* @param {import('./types.js').Signal} signal
*/
function bind_signal_to_component_context(signal) {
if (current_component_context === null || !current_component_context.r) return;
const signals = current_component_context.d;
if (signals) {
signals.push(signal);
} else {
current_component_context.d = [signal];
}
}
/**
* @template V
* @param {V} initial_value
@ -1883,18 +1859,11 @@ export function value_or_fallback(value, fallback) {
/**
* Schedules a callback to run immediately before the component is unmounted.
*
* Out of `onMount`, `beforeUpdate`, `afterUpdate` and `onDestroy`, this is the
* only one that runs inside a server-side component.
*
* https://svelte.dev/docs/svelte#ondestroy
* @param {() => any} fn
* @returns {void}
*/
export function onDestroy(fn) {
if (!is_ssr) {
user_effect(() => () => untrack(fn));
}
function on_destroy(fn) {
user_effect(() => () => untrack(fn));
}
/**
@ -1914,6 +1883,8 @@ export function push(props, runes = false) {
m: false,
// parent
p: current_component_context,
// signals
d: null,
// props
s: props,
// runes
@ -1945,6 +1916,56 @@ export function pop(accessors) {
}
}
/**
* Invoke the getter of all signals associated with a component
* so they can be registered to the effect this function is called in.
* @param {import('./types.js').ComponentContext} context
*/
function observe_all(context) {
if (context.d) {
for (const signal of context.d) get(signal);
}
const props = get_descriptors(context.s);
for (const descriptor of Object.values(props)) {
if (descriptor.get) descriptor.get();
}
}
/**
* Legacy-mode only: Call `onMount` callbacks and set up `beforeUpdate`/`afterUpdate` effects
*/
export function init() {
const context = /** @type {import('./types.js').ComponentContext} */ (current_component_context);
const callbacks = context.u;
if (!callbacks) return;
// beforeUpdate
pre_effect(() => {
observe_all(context);
callbacks.b.forEach(run);
});
// onMount (must run before afterUpdate)
user_effect(() => {
const fns = untrack(() => callbacks.m.map(run));
return () => {
for (const fn of fns) {
if (typeof fn === 'function') {
fn();
}
}
};
});
// afterUpdate
user_effect(() => {
observe_all(context);
callbacks.a.forEach(run);
});
}
/**
* @param {any} value
* @param {Set<any>} visited

@ -36,6 +36,8 @@ export type Store<V> = {
// when the JS VM JITs the code.
export type ComponentContext = {
/** local signals (needed for beforeUpdate/afterUpdate) */
d: null | Signal<any>[];
/** props */
s: Record<string, unknown>;
/** accessors */
@ -52,12 +54,12 @@ export type ComponentContext = {
r: boolean;
/** update_callbacks */
u: null | {
/** before */
b: Array<() => void>;
/** after */
/** afterUpdate callbacks */
a: Array<() => void>;
/** execute */
e: () => void;
/** beforeUpdate callbacks */
b: Array<() => void>;
/** onMount callbacks */
m: Array<() => any>;
};
};
@ -69,8 +71,6 @@ export type ComponentContext = {
export type SourceSignal<V = unknown> = {
/** consumers: Signals that read from the current signal */
c: null | ComputationSignal[];
/** context: The associated component if this signal is an effect/computed */
x: null | ComponentContext;
/** equals: For value equality */
e: null | EqualsFunctions;
/** flags: The types that the signal represent, as a bitwise value */

@ -19,3 +19,8 @@ export function run_all(arr) {
arr[i]();
}
}
/** @param {Function} fn */
export function run(fn) {
return fn();
}

@ -30,7 +30,6 @@ export {
exclude_from_object,
store_set,
unsubscribe_on_destroy,
onDestroy,
pop,
push,
reactive_import,
@ -38,7 +37,8 @@ export {
user_root_effect,
inspect,
unwrap,
freeze
freeze,
init
} from './client/runtime.js';
export * from './client/each.js';
export * from './client/render.js';

@ -1,5 +1,4 @@
import * as $ from '../client/runtime.js';
import { set_is_ssr } from '../client/runtime.js';
import { is_promise, noop } from '../common.js';
import { subscribe_to_store } from '../../store/utils.js';
import { DOMBooleanAttributes } from '../../constants.js';
@ -95,7 +94,6 @@ export function render(component, options) {
const root_anchor = create_anchor(payload);
const root_head_anchor = create_anchor(payload.head);
set_is_ssr(true);
const prev_on_destroy = on_destroy;
on_destroy = [];
payload.out += root_anchor;
@ -112,7 +110,6 @@ export function render(component, options) {
payload.out += root_anchor;
for (const cleanup of on_destroy) cleanup();
on_destroy = prev_on_destroy;
set_is_ssr(false);
return {
head:

@ -1,12 +1,8 @@
import {
current_component_context,
destroy_signal,
get_or_init_context_map,
is_ssr,
managed_effect,
untrack,
user_effect,
flush_local_render_effects
user_effect
} from '../internal/client/runtime.js';
import { is_array } from '../internal/client/utils.js';
@ -25,16 +21,38 @@ import { is_array } from '../internal/client/utils.js';
* @returns {void}
*/
export function onMount(fn) {
if (!is_ssr) {
if (current_component_context === null) {
throw new Error('onMount can only be used during component initialisation.');
}
if (current_component_context.r) {
user_effect(() => {
const result = untrack(fn);
if (typeof result === 'function') {
return /** @type {() => any} */ (result);
}
const cleanup = untrack(fn);
if (typeof cleanup === 'function') return /** @type {() => void} */ (cleanup);
});
} else {
init_update_callbacks(current_component_context).m.push(fn);
}
}
/**
* Schedules a callback to run immediately before the component is unmounted.
*
* Out of `onMount`, `beforeUpdate`, `afterUpdate` and `onDestroy`, this is the
* only one that runs inside a server-side component.
*
* https://svelte.dev/docs/svelte#ondestroy
* @param {() => any} fn
* @returns {void}
*/
export function onDestroy(fn) {
if (current_component_context === null) {
throw new Error('onDestroy can only be used during component initialisation.');
}
onMount(() => () => untrack(fn));
}
/**
* Retrieves the context that belongs to the closest parent component with the specified `key`.
* Must be called during component initialisation.
@ -155,46 +173,6 @@ export function createEventDispatcher() {
};
}
function init_update_callbacks() {
let called_before = false;
let called_after = false;
/** @type {NonNullable<import('../internal/client/types.js').ComponentContext['u']>} */
const update_callbacks = {
b: [],
a: [],
e() {
if (!called_before) {
called_before = true;
// TODO somehow beforeUpdate ran twice on mount in Svelte 4 if it causes a render
// possibly strategy to get this back if needed: analyse beforeUpdate function for assignements to state,
// if yes, add a call to the component to force-run beforeUpdate once.
untrack(() => update_callbacks.b.forEach(/** @param {any} c */ (c) => c()));
flush_local_render_effects();
// beforeUpdate can run again once if afterUpdate causes another update,
// but afterUpdate shouldn't be called again in that case to prevent infinite loops
if (!called_after) {
user_effect(() => {
called_before = false;
called_after = true;
untrack(() => update_callbacks.a.forEach(/** @param {any} c */ (c) => c()));
// managed_effect so that it's not cleaned up when the parent effect is cleaned up
const managed = managed_effect(() => {
destroy_signal(managed);
called_after = false;
});
});
} else {
user_effect(() => {
called_before = false;
});
}
}
}
};
return update_callbacks;
}
// TODO mark beforeUpdate and afterUpdate as deprecated in Svelte 6
/**
@ -210,12 +188,15 @@ function init_update_callbacks() {
* @returns {void}
*/
export function beforeUpdate(fn) {
const component_context = current_component_context;
if (component_context === null) {
throw new Error('beforeUpdate can only be used during component initialisation.');
if (current_component_context === null) {
throw new Error('beforeUpdate can only be used during component initialisation');
}
(component_context.u ??= init_update_callbacks()).b.push(fn);
if (current_component_context.r) {
throw new Error('beforeUpdate cannot be used in runes mode');
}
init_update_callbacks(current_component_context).b.push(fn);
}
/**
@ -231,22 +212,25 @@ export function beforeUpdate(fn) {
* @returns {void}
*/
export function afterUpdate(fn) {
const component_context = current_component_context;
if (component_context === null) {
if (current_component_context === null) {
throw new Error('afterUpdate can only be used during component initialisation.');
}
(component_context.u ??= init_update_callbacks()).a.push(fn);
if (current_component_context.r) {
throw new Error('afterUpdate cannot be used in runes mode');
}
init_update_callbacks(current_component_context).a.push(fn);
}
/**
* Legacy-mode: Init callbacks object for onMount/beforeUpdate/afterUpdate
* @param {import('../internal/client/types.js').ComponentContext} context
*/
function init_update_callbacks(context) {
return (context.u ??= { a: [], b: [], m: [] });
}
// TODO bring implementations in here
// (except probably untrack — do we want to expose that, if there's also a rune?)
export {
flushSync,
createRoot,
mount,
tick,
untrack,
unstate,
onDestroy
} from '../internal/index.js';
export { flushSync, createRoot, mount, tick, untrack, unstate } from '../internal/index.js';

@ -1,4 +1,4 @@
import { noop } from '../internal/common.js';
import { noop, run } from '../internal/common.js';
import { subscribe_to_store } from './utils.js';
/**
@ -106,11 +106,6 @@ export function writable(value, start = noop) {
return { set, update, subscribe };
}
/** @param {Function} fn */
function run(fn) {
return fn();
}
/**
* @param {Function[]} fns
* @returns {void}

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
error: {
code: 'invalid-runes-mode-import',
message: 'beforeUpdate cannot be used in runes mode'
}
});

@ -0,0 +1,5 @@
<svelte:options runes />
<script>
import { beforeUpdate, afterUpdate } from 'svelte';
</script>

@ -112,6 +112,8 @@ async function run_test(
let build_result_ssr;
if (hydrate) {
const ssr_entry = path.resolve(__dirname, '../../src/main/main-server.js');
build_result_ssr = await build({
entryPoints: [`${__dirname}/driver-ssr.js`],
write: false,
@ -123,6 +125,14 @@ async function run_test(
{
name: 'testing-runtime-browser-ssr',
setup(build) {
// When running the server version of the Svelte files,
// we also want to use the server export of the Svelte package
build.onResolve({ filter: /./ }, (args) => {
if (args.path === 'svelte') {
return { path: ssr_entry };
}
});
build.onLoad({ filter: /\.svelte$/ }, (args) => {
const compiled = compile(fs.readFileSync(args.path, 'utf-8').replace(/\r/g, ''), {
generate: 'server',

@ -0,0 +1,25 @@
<script>
import { beforeUpdate, afterUpdate } from "svelte";
import { log } from './log.js';
export let a;
export let b;
beforeUpdate(() => {
log.push('before');
});
beforeUpdate(()=>{
log.push(`before ${a}, ${b}`);
});
afterUpdate(() => {
log.push('after');
});
afterUpdate(()=>{
log.push(`after ${a}, ${b}`);
});
</script>
<p>a: {a}</p>

@ -0,0 +1,45 @@
import { flushSync } from 'svelte';
import { log } from './log.js';
import { test, ok } from '../../test';
export default test({
before_test: () => {
log.length = 0;
},
test({ assert, target }) {
const [button1, button2] = target.querySelectorAll('button');
ok(button1);
ok(button2);
button1.click();
flushSync();
button2.click();
flushSync();
assert.deepEqual(log, [
'before',
'before 0, 0',
'after',
'after 0, 0',
'before',
'before 1, 0',
'after',
'after 1, 0',
'before',
'before 1, 1',
'after',
'after 1, 1'
]);
assert.htmlEqual(
target.innerHTML,
`
<button>a: 1</button>
<button>b: 1</button>
<p>a: 1</p>
`
);
}
});

@ -0,0 +1,10 @@
<script>
import Child from "./Child.svelte";
let a = 0;
let b = 0;
</script>
<button on:click={() => a += 1}>a: {a}</button>
<button on:click={() => b += 1}>b: {b}</button>
<Child {a} {b}/>

@ -46,10 +46,10 @@ export default test({
'2: render 1',
'3: beforeUpdate 1',
'3: render 1',
'parent: afterUpdate 1',
'1: afterUpdate 1',
'2: afterUpdate 1',
'3: afterUpdate 1'
'3: afterUpdate 1',
'parent: afterUpdate 1'
]);
}
});

@ -5,6 +5,7 @@ import * as $ from "svelte/internal";
export default function Each_string_template($$anchor, $$props) {
$.push($$props, false);
$.init();
/* Init */
var fragment = $.comment($$anchor);
@ -27,4 +28,4 @@ export default function Each_string_template($$anchor, $$props) {
$.close_frag($$anchor, fragment);
$.pop();
}
}

@ -7,10 +7,11 @@ var frag = $.template(`<h1>hello world</h1>`);
export default function Hello_world($$anchor, $$props) {
$.push($$props, false);
$.init();
/* Init */
var h1 = $.open($$anchor, true, frag);
$.close($$anchor, h1);
$.pop();
}
}

@ -234,6 +234,15 @@ declare module 'svelte' {
* https://svelte.dev/docs/svelte#onmount
* */
export function onMount<T>(fn: () => NotFunction<T> | Promise<NotFunction<T>> | (() => any)): void;
/**
* Schedules a callback to run immediately before the component is unmounted.
*
* Out of `onMount`, `beforeUpdate`, `afterUpdate` and `onDestroy`, this is the
* only one that runs inside a server-side component.
*
* https://svelte.dev/docs/svelte#ondestroy
* */
export function onDestroy(fn: () => any): void;
/**
* Retrieves the context that belongs to the closest parent component with the specified `key`.
* Must be called during component initialisation.
@ -360,15 +369,6 @@ declare module 'svelte' {
* https://svelte-5-preview.vercel.app/docs/functions#untrack
* */
export function untrack<T>(fn: () => T): T;
/**
* Schedules a callback to run immediately before the component is unmounted.
*
* Out of `onMount`, `beforeUpdate`, `afterUpdate` and `onDestroy`, this is the
* only one that runs inside a server-side component.
*
* https://svelte.dev/docs/svelte#ondestroy
* */
export function onDestroy(fn: () => any): void;
export function unstate<T>(value: T): T;
}

@ -90,10 +90,14 @@ Various error and warning codes have been renamed slightly.
The number of valid namespaces you can pass to the compiler option `namespace` has been reduced to `html` (the default), `svg` and `foreign`.
### beforeUpdate change
### beforeUpdate/afterUpdate changes
`beforeUpdate` no longer runs twice on initial render if it modifies a variable referenced in the template.
`afterUpdate` callbacks in a parent component will now run after `afterUpdate` callbacks in any child components.
Both functions are disallowed in runes mode — use `$effect.pre(...)` and `$effect(...)` instead.
### `contenteditable` behavior change
If you have a `contenteditable` node with a corresponding binding _and_ a reactive value inside it (example: `<div contenteditable=true bind:textContent>count is {count}</div>`), then the value inside the contenteditable will not be updated by updates to `count` because the binding takes full control over the content immediately and it should only be updated through it.

Loading…
Cancel
Save