From c08ecba1b79a0d91f2c69eab6af2fa0dfb8dffd0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 24 Oct 2025 14:12:57 -0400 Subject: [PATCH 1/2] feat: forking (#17004) * chore: run boundary async effects in the context of the current batch * WIP * reinstate kludge * fix test * WIP * WIP * WIP * remove kludge * restore batch_values after commit * make private * tidy up * fix tests * update test * reset #dirty_effects and #maybe_dirty_effects * add test * WIP * add test, fix block resolution * bring async-effect-after-await test from defer-effects-in-pending-boundary branch * avoid reawakening committed batches * changeset * cheat * better API * regenerate * slightly better approach * lint * revert this whatever it is * add test * Update feature description for fork API * error if missing experimental flag * rename inspect effects to eager effects, run them in prod * regenerate * Apply suggestions from code review Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * tidy up * add some minimal prose. probably don't need to go super deep here as it's not really meant for non-framework authors * bit more detail * add a fork_timing error, regenerate * unused * add note * add fork_discarded error * require users to discard forks * add docs * regenerate * tweak docs * fix leak * fix * preload on focusin as well * missed a spot * reduce nesting --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- .changeset/small-geckos-camp.md | 5 + .../19-await-expressions.md | 48 +++++ .../98-reference/.generated/client-errors.md | 18 ++ .../svelte/messages/client-errors/errors.md | 12 ++ packages/svelte/src/index-client.js | 2 +- packages/svelte/src/index-server.js | 4 + packages/svelte/src/index.d.ts | 16 ++ .../svelte/src/internal/client/constants.js | 2 +- .../svelte/src/internal/client/dev/inspect.js | 4 +- .../internal/client/dom/blocks/branches.js | 21 +- .../src/internal/client/dom/blocks/each.js | 2 +- packages/svelte/src/internal/client/errors.js | 48 +++++ packages/svelte/src/internal/client/proxy.js | 8 +- .../src/internal/client/reactivity/batch.js | 182 +++++++++++++++--- .../internal/client/reactivity/deriveds.js | 8 +- .../src/internal/client/reactivity/effects.js | 8 +- .../src/internal/client/reactivity/sources.js | 34 ++-- .../samples/async-fork/_config.js | 92 +++++++++ .../samples/async-fork/main.svelte | 37 ++++ packages/svelte/types/index.d.ts | 44 ++++- 20 files changed, 531 insertions(+), 64 deletions(-) create mode 100644 .changeset/small-geckos-camp.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-fork/main.svelte diff --git a/.changeset/small-geckos-camp.md b/.changeset/small-geckos-camp.md new file mode 100644 index 0000000000..622cbbbfa0 --- /dev/null +++ b/.changeset/small-geckos-camp.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +feat: experimental `fork` API diff --git a/documentation/docs/03-template-syntax/19-await-expressions.md b/documentation/docs/03-template-syntax/19-await-expressions.md index 1c613af870..2f73f6a47c 100644 --- a/documentation/docs/03-template-syntax/19-await-expressions.md +++ b/documentation/docs/03-template-syntax/19-await-expressions.md @@ -135,6 +135,54 @@ If a `` with a `pending` snippet is encountered during SSR, tha > [!NOTE] In the future, we plan to add a streaming implementation that renders the content in the background. +## Forking + +The [`fork(...)`](svelte#fork) API, added in 5.42, makes it possible to run `await` expressions that you _expect_ to happen in the near future. This is mainly intended for frameworks like SvelteKit to implement preloading when (for example) users signal an intent to navigate. + +```svelte + + + + +{#if open} + + open = false} /> +{/if} +``` + ## Caveats As an experimental feature, the details of how `await` is handled (and related APIs like `$effect.pending()`) are subject to breaking changes outside of a semver major release, though we intend to keep such changes to a bare minimum. diff --git a/documentation/docs/98-reference/.generated/client-errors.md b/documentation/docs/98-reference/.generated/client-errors.md index 8fdb7770aa..74a0674dba 100644 --- a/documentation/docs/98-reference/.generated/client-errors.md +++ b/documentation/docs/98-reference/.generated/client-errors.md @@ -130,6 +130,12 @@ $effect(() => { Often when encountering this issue, the value in question shouldn't be state (for example, if you are pushing to a `logs` array in an effect, make `logs` a normal array rather than `$state([])`). In the rare cases where you really _do_ need to write to state in an effect — [which you should avoid]($effect#When-not-to-use-$effect) — you can read the state with [untrack](svelte#untrack) to avoid adding it as a dependency. +### experimental_async_fork + +``` +Cannot use `fork(...)` unless the `experimental.async` compiler option is `true` +``` + ### flush_sync_in_effect ``` @@ -140,6 +146,18 @@ The `flushSync()` function can be used to flush any pending effects synchronousl This restriction only applies when using the `experimental.async` option, which will be active by default in Svelte 6. +### fork_discarded + +``` +Cannot commit a fork that was already committed or discarded +``` + +### fork_timing + +``` +Cannot create a fork inside an effect or when state changes are pending +``` + ### get_abort_signal_outside_reaction ``` diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index 57ecca0489..b5fe51539d 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -100,6 +100,10 @@ $effect(() => { Often when encountering this issue, the value in question shouldn't be state (for example, if you are pushing to a `logs` array in an effect, make `logs` a normal array rather than `$state([])`). In the rare cases where you really _do_ need to write to state in an effect — [which you should avoid]($effect#When-not-to-use-$effect) — you can read the state with [untrack](svelte#untrack) to avoid adding it as a dependency. +## experimental_async_fork + +> Cannot use `fork(...)` unless the `experimental.async` compiler option is `true` + ## flush_sync_in_effect > Cannot use `flushSync` inside an effect @@ -108,6 +112,14 @@ The `flushSync()` function can be used to flush any pending effects synchronousl This restriction only applies when using the `experimental.async` option, which will be active by default in Svelte 6. +## fork_discarded + +> Cannot commit a fork that was already committed or discarded + +## fork_timing + +> Cannot create a fork inside an effect or when state changes are pending + ## get_abort_signal_outside_reaction > `getAbortSignal()` can only be called inside an effect or derived diff --git a/packages/svelte/src/index-client.js b/packages/svelte/src/index-client.js index 337cbb500b..4fcfff980d 100644 --- a/packages/svelte/src/index-client.js +++ b/packages/svelte/src/index-client.js @@ -241,7 +241,7 @@ function init_update_callbacks(context) { return (l.u ??= { a: [], b: [], m: [] }); } -export { flushSync } from './internal/client/reactivity/batch.js'; +export { flushSync, fork } from './internal/client/reactivity/batch.js'; export { createContext, getContext, diff --git a/packages/svelte/src/index-server.js b/packages/svelte/src/index-server.js index 223ce6a4cd..61b0d98c06 100644 --- a/packages/svelte/src/index-server.js +++ b/packages/svelte/src/index-server.js @@ -33,6 +33,10 @@ export function unmount() { e.lifecycle_function_unavailable('unmount'); } +export function fork() { + e.lifecycle_function_unavailable('fork'); +} + export async function tick() {} export async function settled() {} diff --git a/packages/svelte/src/index.d.ts b/packages/svelte/src/index.d.ts index 38e6086689..a1782f5b61 100644 --- a/packages/svelte/src/index.d.ts +++ b/packages/svelte/src/index.d.ts @@ -352,4 +352,20 @@ export type MountOptions = Record props: Props; }); +/** + * Represents work that is happening off-screen, such as data being preloaded + * in anticipation of the user navigating + * @since 5.42 + */ +export interface Fork { + /** + * Commit the fork. The promise will resolve once the state change has been applied + */ + commit(): Promise; + /** + * Discard the fork + */ + discard(): void; +} + export * from './index-client.js'; diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 6818fd9d30..24dc9e4fb8 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -19,7 +19,7 @@ export const EFFECT_RAN = 1 << 15; * This is on a block effect 99% of the time but may also be on a branch effect if its parent block effect was pruned */ export const EFFECT_TRANSPARENT = 1 << 16; -export const INSPECT_EFFECT = 1 << 17; +export const EAGER_EFFECT = 1 << 17; export const HEAD_EFFECT = 1 << 18; export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; diff --git a/packages/svelte/src/internal/client/dev/inspect.js b/packages/svelte/src/internal/client/dev/inspect.js index db7ab0d976..09150d6ee4 100644 --- a/packages/svelte/src/internal/client/dev/inspect.js +++ b/packages/svelte/src/internal/client/dev/inspect.js @@ -1,6 +1,6 @@ import { UNINITIALIZED } from '../../../constants.js'; import { snapshot } from '../../shared/clone.js'; -import { inspect_effect, render_effect, validate_effect } from '../reactivity/effects.js'; +import { eager_effect, render_effect, validate_effect } from '../reactivity/effects.js'; import { untrack } from '../runtime.js'; import { get_stack } from './tracing.js'; @@ -19,7 +19,7 @@ export function inspect(get_value, inspector, show_stack = false) { // stack traces. As a consequence, reading the value might result // in an error (an `$inspect(object.property)` will run before the // `{#if object}...{/if}` that contains it) - inspect_effect(() => { + eager_effect(() => { try { var value = get_value(); } catch (e) { diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index 827f9f44fa..f1b9baf6f6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/branches.js +++ b/packages/svelte/src/internal/client/dom/blocks/branches.js @@ -1,5 +1,4 @@ /** @import { Effect, TemplateNode } from '#client' */ -import { is_runes } from '../../context.js'; import { Batch, current_batch } from '../../reactivity/batch.js'; import { branch, @@ -8,7 +7,6 @@ import { pause_effect, resume_effect } from '../../reactivity/effects.js'; -import { set_should_intro, should_intro } from '../../render.js'; import { hydrate_node, hydrating } from '../hydration.js'; import { create_text, should_defer_append } from '../operations.js'; @@ -126,6 +124,22 @@ export class BranchManager { } }; + /** + * @param {Batch} batch + */ + #discard = (batch) => { + this.#batches.delete(batch); + + const keys = Array.from(this.#batches.values()); + + for (const [k, branch] of this.#offscreen) { + if (!keys.includes(k)) { + destroy_effect(branch.effect); + this.#offscreen.delete(k); + } + } + }; + /** * * @param {any} key @@ -173,7 +187,8 @@ export class BranchManager { } } - batch.add_callback(this.#commit); + batch.oncommit(this.#commit); + batch.ondiscard(this.#discard); } else { if (hydrating) { this.anchor = hydrate_node; diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index a6369a7211..a0fae37133 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -310,7 +310,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } } - batch.add_callback(commit); + batch.oncommit(commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index 937971da5e..2a433ed8f9 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -229,6 +229,22 @@ export function effect_update_depth_exceeded() { } } +/** + * Cannot use `fork(...)` unless the `experimental.async` compiler option is `true` + * @returns {never} + */ +export function experimental_async_fork() { + if (DEV) { + const error = new Error(`experimental_async_fork\nCannot use \`fork(...)\` unless the \`experimental.async\` compiler option is \`true\`\nhttps://svelte.dev/e/experimental_async_fork`); + + error.name = 'Svelte error'; + + throw error; + } else { + throw new Error(`https://svelte.dev/e/experimental_async_fork`); + } +} + /** * Cannot use `flushSync` inside an effect * @returns {never} @@ -245,6 +261,38 @@ export function flush_sync_in_effect() { } } +/** + * Cannot commit a fork that was already committed or discarded + * @returns {never} + */ +export function fork_discarded() { + if (DEV) { + const error = new Error(`fork_discarded\nCannot commit a fork that was already committed or discarded\nhttps://svelte.dev/e/fork_discarded`); + + error.name = 'Svelte error'; + + throw error; + } else { + throw new Error(`https://svelte.dev/e/fork_discarded`); + } +} + +/** + * Cannot create a fork inside an effect or when state changes are pending + * @returns {never} + */ +export function fork_timing() { + if (DEV) { + const error = new Error(`fork_timing\nCannot create a fork inside an effect or when state changes are pending\nhttps://svelte.dev/e/fork_timing`); + + error.name = 'Svelte error'; + + throw error; + } else { + throw new Error(`https://svelte.dev/e/fork_timing`); + } +} + /** * `getAbortSignal()` can only be called inside an effect or derived * @returns {never} diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index dae3791eb0..9baacacd0d 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -19,8 +19,8 @@ import { state as source, set, increment, - flush_inspect_effects, - set_inspect_effects_deferred + flush_eager_effects, + set_eager_effects_deferred } from './reactivity/sources.js'; import { PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants'; import { UNINITIALIZED } from '../../constants.js'; @@ -421,9 +421,9 @@ function inspectable_array(array) { * @param {any[]} args */ return function (...args) { - set_inspect_effects_deferred(); + set_eager_effects_deferred(); var result = value.apply(this, args); - flush_inspect_effects(); + flush_eager_effects(); return result; }; } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 2cef562ac9..fdeb111a4d 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -1,3 +1,4 @@ +/** @import { Fork } from 'svelte' */ /** @import { Derived, Effect, Reaction, Source, Value } from '#client' */ import { BLOCK_EFFECT, @@ -12,25 +13,35 @@ import { ROOT_EFFECT, MAYBE_DIRTY, DERIVED, - BOUNDARY_EFFECT + BOUNDARY_EFFECT, + EAGER_EFFECT } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property } from '../../shared/utils.js'; import { active_effect, get, + increment_write_version, is_dirty, is_updating_effect, set_is_updating_effect, set_signal_status, + tick, update_effect } from '../runtime.js'; import * as e from '../errors.js'; import { flush_tasks, queue_micro_task } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; -import { old_values, source, update } from './sources.js'; -import { inspect_effect, unlink_effect } from './effects.js'; +import { + flush_eager_effects, + eager_effects, + old_values, + set_eager_effects, + source, + update +} from './sources.js'; +import { eager_effect, unlink_effect } from './effects.js'; /** * @typedef {{ @@ -90,14 +101,20 @@ export class Batch { * They keys of this map are identical to `this.#current` * @type {Map} */ - #previous = new Map(); + previous = new Map(); /** * When the batch is committed (and the DOM is updated), we need to remove old branches * and append new ones by calling the functions added inside (if/each/key/etc) blocks * @type {Set<() => void>} */ - #callbacks = new Set(); + #commit_callbacks = new Set(); + + /** + * If a fork is discarded, we need to destroy any effects that are no longer needed + * @type {Set<(batch: Batch) => void>} + */ + #discard_callbacks = new Set(); /** * The number of async effects that are currently in flight @@ -135,6 +152,8 @@ export class Batch { */ skipped_effects = new Set(); + is_fork = false; + /** * * @param {Effect[]} root_effects @@ -159,15 +178,15 @@ export class Batch { this.#traverse_effect_tree(root, target); } - this.#resolve(); + if (!this.is_fork) { + this.#resolve(); + } - if (this.#blocking_pending > 0) { + if (this.#blocking_pending > 0 || this.is_fork) { this.#defer_effects(target.effects); this.#defer_effects(target.render_effects); this.#defer_effects(target.block_effects); } else { - // TODO append/detach blocks here, not in #commit - // If sources are written to, then work needs to happen in a separate batch, else prior sources would be mixed with // newly updated sources, which could lead to infinite loops when effects run over and over again. previous_batch = this; @@ -271,8 +290,8 @@ export class Batch { * @param {any} value */ capture(source, value) { - if (!this.#previous.has(source)) { - this.#previous.set(source, value); + if (!this.previous.has(source)) { + this.previous.set(source, value); } this.current.set(source, source.v); @@ -289,16 +308,17 @@ export class Batch { } flush() { + this.activate(); + if (queued_root_effects.length > 0) { - this.activate(); flush_effects(); if (current_batch !== null && current_batch !== this) { // this can happen if a new batch was created during `flush_effects()` return; } - } else { - this.#resolve(); + } else if (this.#pending === 0) { + this.process([]); // TODO this feels awkward } this.deactivate(); @@ -314,11 +334,16 @@ export class Batch { } } + discard() { + for (const fn of this.#discard_callbacks) fn(this); + this.#discard_callbacks.clear(); + } + #resolve() { if (this.#blocking_pending === 0) { // append/remove branches - for (const fn of this.#callbacks) fn(); - this.#callbacks.clear(); + for (const fn of this.#commit_callbacks) fn(); + this.#commit_callbacks.clear(); } if (this.#pending === 0) { @@ -332,7 +357,7 @@ export class Batch { // committed state, unless the batch in question has a more // recent value for a given source if (batches.size > 1) { - this.#previous.clear(); + this.previous.clear(); var previous_batch_values = batch_values; var is_earlier = true; @@ -428,6 +453,10 @@ export class Batch { this.#pending -= 1; if (blocking) this.#blocking_pending -= 1; + this.revive(); + } + + revive() { for (const e of this.#dirty_effects) { set_signal_status(e, DIRTY); schedule_effect(e); @@ -445,8 +474,13 @@ export class Batch { } /** @param {() => void} fn */ - add_callback(fn) { - this.#callbacks.add(fn); + oncommit(fn) { + this.#commit_callbacks.add(fn); + } + + /** @param {(batch: Batch) => void} fn */ + ondiscard(fn) { + this.#discard_callbacks.add(fn); } settled() { @@ -489,7 +523,7 @@ export class Batch { for (const batch of batches) { if (batch === this) continue; - for (const [source, previous] of batch.#previous) { + for (const [source, previous] of batch.previous) { if (!batch_values.has(source)) { batch_values.set(source, previous); } @@ -717,6 +751,28 @@ function mark_effects(value, sources, marked, checked) { } } +/** + * When committing a fork, we need to trigger eager effects so that + * any `$state.eager(...)` expressions update immediately. This + * function allows us to discover them + * @param {Value} value + * @param {Set} effects + */ +function mark_eager_effects(value, effects) { + if (value.reactions === null) return; + + for (const reaction of value.reactions) { + const flags = reaction.f; + + if ((flags & DERIVED) !== 0) { + mark_eager_effects(/** @type {Derived} */ (reaction), effects); + } else if ((flags & EAGER_EFFECT) !== 0) { + set_signal_status(reaction, DIRTY); + effects.add(/** @type {Effect} */ (reaction)); + } + } +} + /** * @param {Reaction} reaction * @param {Source[]} sources @@ -798,9 +854,9 @@ export function eager(fn) { get(version); - inspect_effect(() => { + eager_effect(() => { if (initial) { - // the first time this runs, we create an inspect effect + // the first time this runs, we create an eager effect // that will run eagerly whenever the expression changes var previous_batch_values = batch_values; @@ -829,6 +885,88 @@ export function eager(fn) { return value; } +/** + * Creates a 'fork', in which state changes are evaluated but not applied to the DOM. + * This is useful for speculatively loading data (for example) when you suspect that + * the user is about to take some action. + * + * Frameworks like SvelteKit can use this to preload data when the user touches or + * hovers over a link, making any subsequent navigation feel instantaneous. + * + * The `fn` parameter is a synchronous function that modifies some state. The + * state changes will be reverted after the fork is initialised, then reapplied + * if and when the fork is eventually committed. + * + * When it becomes clear that a fork will _not_ be committed (e.g. because the + * user navigated elsewhere), it must be discarded to avoid leaking memory. + * + * @param {() => void} fn + * @returns {Fork} + * @since 5.42 + */ +export function fork(fn) { + if (!async_mode_flag) { + e.experimental_async_fork(); + } + + if (current_batch !== null) { + e.fork_timing(); + } + + const batch = Batch.ensure(); + batch.is_fork = true; + + const settled = batch.settled(); + + flushSync(fn); + + // revert state changes + for (const [source, value] of batch.previous) { + source.v = value; + } + + return { + commit: async () => { + if (!batches.has(batch)) { + e.fork_discarded(); + } + + batch.is_fork = false; + + // apply changes + for (const [source, value] of batch.current) { + source.v = value; + } + + // trigger any `$state.eager(...)` expressions with the new state. + // eager effects don't get scheduled like other effects, so we + // can't just encounter them during traversal, we need to + // proactively flush them + // TODO maybe there's a better implementation? + flushSync(() => { + /** @type {Set} */ + const eager_effects = new Set(); + + for (const source of batch.current.keys()) { + mark_eager_effects(source, eager_effects); + } + + set_eager_effects(eager_effects); + flush_eager_effects(); + }); + + batch.revive(); + await settled; + }, + discard: () => { + if (batches.has(batch)) { + batches.delete(batch); + batch.discard(); + } + } + }; +} + /** * Forcibly remove all current batches, to prevent cross-talk between tests */ diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 5a3dee4b7f..b6a50acc4d 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -28,7 +28,7 @@ import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; import * as w from '../warnings.js'; import { async_effect, destroy_effect, teardown } from './effects.js'; -import { inspect_effects, internal_set, set_inspect_effects, source } from './sources.js'; +import { eager_effects, internal_set, set_eager_effects, source } from './sources.js'; import { get_stack } from '../dev/tracing.js'; import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { Boundary } from '../dom/blocks/boundary.js'; @@ -318,8 +318,8 @@ export function execute_derived(derived) { set_active_effect(get_derived_parent_effect(derived)); if (DEV) { - let prev_inspect_effects = inspect_effects; - set_inspect_effects(new Set()); + let prev_eager_effects = eager_effects; + set_eager_effects(new Set()); try { if (stack.includes(derived)) { e.derived_references_self(); @@ -332,7 +332,7 @@ export function execute_derived(derived) { value = update_reaction(derived); } finally { set_active_effect(prev_active_effect); - set_inspect_effects(prev_inspect_effects); + set_eager_effects(prev_eager_effects); stack.pop(); } } else { diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 9b54598f9e..4235e9cb24 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -27,7 +27,7 @@ import { DERIVED, UNOWNED, CLEAN, - INSPECT_EFFECT, + EAGER_EFFECT, HEAD_EFFECT, MAYBE_DIRTY, EFFECT_PRESERVED, @@ -88,7 +88,7 @@ function create_effect(type, fn, sync, push = true) { if (DEV) { // Ensure the parent is never an inspect effect - while (parent !== null && (parent.f & INSPECT_EFFECT) !== 0) { + while (parent !== null && (parent.f & EAGER_EFFECT) !== 0) { parent = parent.parent; } } @@ -245,8 +245,8 @@ export function user_pre_effect(fn) { } /** @param {() => void | (() => void)} fn */ -export function inspect_effect(fn) { - return create_effect(INSPECT_EFFECT, fn, true); +export function eager_effect(fn) { + return create_effect(EAGER_EFFECT, fn, true); } /** diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 2fe8c4f75d..9534e718a5 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -22,7 +22,7 @@ import { DERIVED, DIRTY, BRANCH_EFFECT, - INSPECT_EFFECT, + EAGER_EFFECT, UNOWNED, MAYBE_DIRTY, BLOCK_EFFECT, @@ -39,7 +39,7 @@ import { proxy } from '../proxy.js'; import { execute_derived } from './deriveds.js'; /** @type {Set} */ -export let inspect_effects = new Set(); +export let eager_effects = new Set(); /** @type {Map} */ export const old_values = new Map(); @@ -47,14 +47,14 @@ export const old_values = new Map(); /** * @param {Set} v */ -export function set_inspect_effects(v) { - inspect_effects = v; +export function set_eager_effects(v) { + eager_effects = v; } -let inspect_effects_deferred = false; +let eager_effects_deferred = false; -export function set_inspect_effects_deferred() { - inspect_effects_deferred = true; +export function set_eager_effects_deferred() { + eager_effects_deferred = true; } /** @@ -146,9 +146,9 @@ export function set(source, value, should_proxy = false) { active_reaction !== null && // since we are untracking the function inside `$inspect.with` we need to add this check // to ensure we error if state is set inside an inspect effect - (!untracking || (active_reaction.f & INSPECT_EFFECT) !== 0) && + (!untracking || (active_reaction.f & EAGER_EFFECT) !== 0) && is_runes() && - (active_reaction.f & (DERIVED | BLOCK_EFFECT | ASYNC | INSPECT_EFFECT)) !== 0 && + (active_reaction.f & (DERIVED | BLOCK_EFFECT | ASYNC | EAGER_EFFECT)) !== 0 && !current_sources?.includes(source) ) { e.state_unsafe_mutation(); @@ -235,18 +235,18 @@ export function internal_set(source, value) { } } - if (DEV && inspect_effects.size > 0 && !inspect_effects_deferred) { - flush_inspect_effects(); + if (!batch.is_fork && eager_effects.size > 0 && !eager_effects_deferred) { + flush_eager_effects(); } } return value; } -export function flush_inspect_effects() { - inspect_effects_deferred = false; +export function flush_eager_effects() { + eager_effects_deferred = false; - const inspects = Array.from(inspect_effects); + const inspects = Array.from(eager_effects); for (const effect of inspects) { // Mark clean inspect-effects as maybe dirty and then check their dirtiness @@ -260,7 +260,7 @@ export function flush_inspect_effects() { } } - inspect_effects.clear(); + eager_effects.clear(); } /** @@ -320,8 +320,8 @@ function mark_reactions(signal, status) { if (!runes && reaction === active_effect) continue; // Inspect effects need to run immediately, so that the stack trace makes sense - if (DEV && (flags & INSPECT_EFFECT) !== 0) { - inspect_effects.add(reaction); + if (DEV && (flags & EAGER_EFFECT) !== 0) { + eager_effects.add(reaction); continue; } diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork/_config.js b/packages/svelte/tests/runtime-runes/samples/async-fork/_config.js new file mode 100644 index 0000000000..35b47525a2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork/_config.js @@ -0,0 +1,92 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, raf }) { + const [shift, increment, commit] = target.querySelectorAll('button'); + + shift.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + + +

count: 0

+

eager: 0

+

even

+ ` + ); + + increment.click(); + await tick(); + + shift.click(); + await tick(); + + // nothing updates until commit + assert.htmlEqual( + target.innerHTML, + ` + + + +

count: 0

+

eager: 0

+

even

+ ` + ); + + commit.click(); + await tick(); + + // nothing updates until commit + assert.htmlEqual( + target.innerHTML, + ` + + + +

count: 1

+

eager: 1

+

odd

+ ` + ); + + increment.click(); + await tick(); + + commit.click(); + await tick(); + + // eager state updates on commit + assert.htmlEqual( + target.innerHTML, + ` + + + +

count: 1

+

eager: 2

+

odd

+ ` + ); + + shift.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + + +

count: 2

+

eager: 2

+

even

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-fork/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-fork/main.svelte new file mode 100644 index 0000000000..9b1f433f2d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-fork/main.svelte @@ -0,0 +1,37 @@ + + + + + + +

count: {count}

+

eager: {$state.eager(count)}

+ + + {#if await push(count) % 2 === 0} +

even

+ {:else} +

odd

+ {/if} + + {#snippet pending()} +

loading...

+ {/snippet} +
diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index d260b738c3..5e3ca77eb5 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -348,6 +348,22 @@ declare module 'svelte' { */ props: Props; }); + + /** + * Represents work that is happening off-screen, such as data being preloaded + * in anticipation of the user navigating + * @since 5.42 + */ + export interface Fork { + /** + * Commit the fork. The promise will resolve once the state change has been applied + */ + commit(): Promise; + /** + * Discard the fork + */ + discard(): void; + } /** * Returns an [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) that aborts when the current [derived](https://svelte.dev/docs/svelte/$derived) or [effect](https://svelte.dev/docs/svelte/$effect) re-runs or is destroyed. * @@ -434,11 +450,6 @@ declare module 'svelte' { * @deprecated Use [`$effect`](https://svelte.dev/docs/svelte/$effect) instead * */ export function afterUpdate(fn: () => void): void; - /** - * Synchronously flush any pending updates. - * Returns void if no callback is provided, otherwise returns the result of calling the callback. - * */ - export function flushSync(fn?: (() => T) | undefined): T; /** * Create a snippet programmatically * */ @@ -448,6 +459,29 @@ declare module 'svelte' { }): Snippet; /** Anything except a function */ type NotFunction = T extends Function ? never : T; + /** + * Synchronously flush any pending updates. + * Returns void if no callback is provided, otherwise returns the result of calling the callback. + * */ + export function flushSync(fn?: (() => T) | undefined): T; + /** + * Creates a 'fork', in which state changes are evaluated but not applied to the DOM. + * This is useful for speculatively loading data (for example) when you suspect that + * the user is about to take some action. + * + * Frameworks like SvelteKit can use this to preload data when the user touches or + * hovers over a link, making any subsequent navigation feel instantaneous. + * + * The `fn` parameter is a synchronous function that modifies some state. The + * state changes will be reverted after the fork is initialised, then reapplied + * if and when the fork is eventually committed. + * + * When it becomes clear that a fork will _not_ be committed (e.g. because the + * user navigated elsewhere), it must be discarded to avoid leaking memory. + * + * @since 5.42 + */ + export function fork(fn: () => void): Fork; /** * Returns a `[get, set]` pair of functions for working with context in a type-safe way. * From 4eb432e941d4d8ebf1206ea86433b0c778b71737 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 24 Oct 2025 17:27:04 -0400 Subject: [PATCH 2/2] chore: remove event hoisting (#17030) * chore: get rid of hoisted event handlers * remove unused stuff * simplify * wow we can delete so much more code. this makes me so happy * even more! --- .../phases/2-analyze/visitors/Attribute.js | 188 +----------------- .../2-analyze/visitors/shared/function.js | 7 - .../phases/3-transform/client/utils.js | 126 +----------- .../client/visitors/FunctionDeclaration.js | 11 - .../client/visitors/VariableDeclaration.js | 18 -- .../client/visitors/shared/events.js | 30 +-- .../client/visitors/shared/function.js | 13 -- .../src/compiler/phases/3-transform/utils.js | 19 -- packages/svelte/src/compiler/phases/nodes.js | 2 +- .../svelte/src/compiler/phases/types.d.ts | 26 --- .../svelte/src/compiler/types/template.d.ts | 11 +- .../svelte/src/compiler/utils/builders.js | 9 +- .../client/dom/elements/attributes.js | 4 +- .../internal/client/dom/elements/events.js | 9 +- packages/svelte/src/utils.js | 2 +- .../samples/inspect-new-property/_config.js | 4 +- .../samples/inspect-recursive/_config.js | 4 +- .../_expected/client/index.svelte.js | 11 +- .../_expected/client/index.svelte.js | 14 +- .../_expected/client/index.svelte.js | 3 +- .../_expected/client/index.svelte.js | 17 +- 21 files changed, 45 insertions(+), 483 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index b13f3f89b6..2b7d636606 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -1,12 +1,7 @@ -/** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression } from 'estree' */ -/** @import { AST, DelegatedEvent } from '#compiler' */ +/** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ -import { cannot_be_set_statically, is_capture_event, is_delegated } from '../../../../utils.js'; -import { - get_attribute_chunks, - get_attribute_expression, - is_event_attribute -} from '../../../utils/ast.js'; +import { cannot_be_set_statically, can_delegate_event } from '../../../../utils.js'; +import { get_attribute_chunks, is_event_attribute } from '../../../utils/ast.js'; import { mark_subtree_dynamic } from './shared/fragment.js'; /** @@ -64,181 +59,8 @@ export function Attribute(node, context) { context.state.analysis.uses_event_attributes = true; } - const expression = get_attribute_expression(node); - const delegated_event = get_delegated_event(node.name.slice(2), expression, context); - - if (delegated_event !== null) { - if (delegated_event.hoisted) { - delegated_event.function.metadata.hoisted = true; - } - - node.metadata.delegated = delegated_event; - } - } - } -} - -/** @type {DelegatedEvent} */ -const unhoisted = { hoisted: false }; - -/** - * Checks if given event attribute can be delegated/hoisted and returns the corresponding info if so - * @param {string} event_name - * @param {Expression | null} handler - * @param {Context} context - * @returns {null | DelegatedEvent} - */ -function get_delegated_event(event_name, handler, context) { - // Handle delegated event handlers. Bail out if not a delegated event. - if (!handler || !is_delegated(event_name)) { - return null; - } - - // If we are not working with a RegularElement, then bail out. - const element = context.path.at(-1); - if (element?.type !== 'RegularElement') { - return null; - } - - /** @type {FunctionExpression | FunctionDeclaration | ArrowFunctionExpression | null} */ - let target_function = null; - let binding = null; - - if (element.metadata.has_spread) { - // event attribute becomes part of the dynamic spread array - return unhoisted; - } - - if (handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') { - target_function = handler; - } else if (handler.type === 'Identifier') { - binding = context.state.scope.get(handler.name); - - if (context.state.analysis.module.scope.references.has(handler.name)) { - // If a binding with the same name is referenced in the module scope (even if not declared there), bail out - return unhoisted; - } - - if (binding != null) { - for (const { path } of binding.references) { - const parent = path.at(-1); - if (parent === undefined) return unhoisted; - - const grandparent = path.at(-2); - - /** @type {AST.RegularElement | null} */ - let element = null; - /** @type {string | null} */ - let event_name = null; - if (parent.type === 'OnDirective') { - element = /** @type {AST.RegularElement} */ (grandparent); - event_name = parent.name; - } else if ( - parent.type === 'ExpressionTag' && - grandparent?.type === 'Attribute' && - is_event_attribute(grandparent) - ) { - element = /** @type {AST.RegularElement} */ (path.at(-3)); - const attribute = /** @type {AST.Attribute} */ (grandparent); - event_name = get_attribute_event_name(attribute.name); - } - - if (element && event_name) { - if ( - element.type !== 'RegularElement' || - element.metadata.has_spread || - !is_delegated(event_name) - ) { - return unhoisted; - } - } else if (parent.type !== 'FunctionDeclaration' && parent.type !== 'VariableDeclarator') { - return unhoisted; - } - } + node.metadata.delegated = + parent?.type === 'RegularElement' && can_delegate_event(node.name.slice(2)); } - - // If the binding is exported, bail out - if (context.state.analysis.exports.find((node) => node.name === handler.name)) { - return unhoisted; - } - - if (binding?.is_function()) { - target_function = binding.initial; - } - } - - // If we can't find a function, or the function has multiple parameters, bail out - if (target_function == null || target_function.params.length > 1) { - return unhoisted; - } - - const visited_references = new Set(); - const scope = target_function.metadata.scope; - for (const [reference] of scope.references) { - // Bail out if the arguments keyword is used or $host is referenced - if (reference === 'arguments' || reference === '$host') return unhoisted; - // Bail out if references a store subscription - if (scope.get(`$${reference}`)?.kind === 'store_sub') return unhoisted; - - const binding = scope.get(reference); - const local_binding = context.state.scope.get(reference); - - // if the function access a snippet that can't be hoisted we bail out - if ( - local_binding !== null && - local_binding.initial?.type === 'SnippetBlock' && - !local_binding.initial.metadata.can_hoist - ) { - return unhoisted; - } - - // If we are referencing a binding that is shadowed in another scope then bail out (unless it's declared within the function). - if ( - local_binding !== null && - binding !== null && - local_binding.node !== binding.node && - scope.declarations.get(reference) !== binding - ) { - return unhoisted; - } - - // If we have multiple references to the same store using $ prefix, bail out. - if ( - binding !== null && - binding.kind === 'store_sub' && - visited_references.has(reference.slice(1)) - ) { - return unhoisted; - } - - // If we reference the index within an each block, then bail out. - if (binding !== null && binding.initial?.type === 'EachBlock') return unhoisted; - - if ( - binding !== null && - // Bail out if the binding is a rest param - (binding.declaration_kind === 'rest_param' || - // Bail out if we reference anything from the EachBlock (for now) that mutates in non-runes mode, - (((!context.state.analysis.runes && binding.kind === 'each') || - // or any normal not reactive bindings that are mutated. - binding.kind === 'normal') && - binding.updated)) - ) { - return unhoisted; - } - visited_references.add(reference); - } - - return { hoisted: true, function: target_function }; -} - -/** - * @param {string} event_name - */ -function get_attribute_event_name(event_name) { - event_name = event_name.slice(2); - if (is_capture_event(event_name)) { - event_name = event_name.slice(0, -7); } - return event_name; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js index 1776167850..4d93cd44e0 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js @@ -6,13 +6,6 @@ * @param {Context} context */ export function visit_function(node, context) { - // TODO retire this in favour of a more general solution based on bindings - node.metadata = { - hoisted: false, - hoisted_params: [], - scope: context.state.scope - }; - if (context.state.expression) { for (const [name] of context.state.scope.references) { const binding = context.state.scope.get(name); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 41ed277898..f21fb43fc1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -1,6 +1,6 @@ -/** @import { ArrowFunctionExpression, AssignmentExpression, BlockStatement, Expression, FunctionDeclaration, FunctionExpression, Identifier, Node, Pattern, UpdateExpression } from 'estree' */ +/** @import { BlockStatement, Expression, Identifier } from 'estree' */ /** @import { Binding } from '#compiler' */ -/** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */ +/** @import { ClientTransformState, ComponentClientTransformState } from './types.js' */ /** @import { Analysis } from '../../types.js' */ /** @import { Scope } from '../../scope.js' */ import * as b from '#compiler/builders'; @@ -12,9 +12,6 @@ import { PROPS_IS_UPDATED, PROPS_IS_BINDABLE } from '../../../../constants.js'; -import { dev } from '../../../state.js'; -import { walk } from 'zimmerframe'; -import { validate_mutation } from './visitors/shared/utils.js'; /** * @param {Binding} binding @@ -46,125 +43,6 @@ export function build_getter(node, state) { return node; } -/** - * @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression} node - * @param {ComponentContext} context - * @returns {Pattern[]} - */ -function get_hoisted_params(node, context) { - const scope = context.state.scope; - - /** @type {Identifier[]} */ - const params = []; - - /** - * We only want to push if it's not already present to avoid name clashing - * @param {Identifier} id - */ - function push_unique(id) { - if (!params.find((param) => param.name === id.name)) { - params.push(id); - } - } - - for (const [reference] of scope.references) { - let binding = scope.get(reference); - - if (binding !== null && !scope.declarations.has(reference) && binding.initial !== node) { - if (binding.kind === 'store_sub') { - // We need both the subscription for getting the value and the store for updating - push_unique(b.id(binding.node.name)); - binding = /** @type {Binding} */ (scope.get(binding.node.name.slice(1))); - } - - let expression = context.state.transform[reference]?.read(b.id(binding.node.name)); - - if ( - // If it's a destructured derived binding, then we can extract the derived signal reference and use that. - // TODO this code is bad, we need to kill it - expression != null && - typeof expression !== 'function' && - expression.type === 'MemberExpression' && - expression.object.type === 'CallExpression' && - expression.object.callee.type === 'Identifier' && - expression.object.callee.name === '$.get' && - expression.object.arguments[0].type === 'Identifier' - ) { - push_unique(b.id(expression.object.arguments[0].name)); - } else if ( - // If we are referencing a simple $$props value, then we need to reference the object property instead - (binding.kind === 'prop' || binding.kind === 'bindable_prop') && - !is_prop_source(binding, context.state) - ) { - push_unique(b.id('$$props')); - } else if ( - // imports don't need to be hoisted - binding.declaration_kind !== 'import' - ) { - // create a copy to remove start/end tags which would mess up source maps - push_unique(b.id(binding.node.name)); - // rest props are often accessed through the $$props object for optimization reasons, - // but we can't know if the delegated event handler will use it, so we need to add both as params - if (binding.kind === 'rest_prop' && context.state.analysis.runes) { - push_unique(b.id('$$props')); - } - } - } - } - - if (dev) { - // this is a little hacky, but necessary for ownership validation - // to work inside hoisted event handlers - - /** - * @param {AssignmentExpression | UpdateExpression} node - * @param {{ next: () => void, stop: () => void }} context - */ - function visit(node, { next, stop }) { - if (validate_mutation(node, /** @type {any} */ (context), node) !== node) { - params.push(b.id('$$ownership_validator')); - stop(); - } else { - next(); - } - } - - walk(/** @type {Node} */ (node), null, { - AssignmentExpression: visit, - UpdateExpression: visit - }); - } - - return params; -} - -/** - * @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression} node - * @param {ComponentContext} context - * @returns {Pattern[]} - */ -export function build_hoisted_params(node, context) { - const hoisted_params = get_hoisted_params(node, context); - node.metadata.hoisted_params = hoisted_params; - - /** @type {Pattern[]} */ - const params = []; - - if (node.params.length === 0) { - if (hoisted_params.length > 0) { - // For the event object - params.push(b.id(context.state.scope.generate('_'))); - } - } else { - for (const param of node.params) { - params.push(/** @type {Pattern} */ (context.visit(param))); - } - } - - params.push(...hoisted_params); - return params; -} - /** * @param {Binding} binding * @param {ComponentClientTransformState} state diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js index cd299a710b..17327c21d6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js @@ -1,7 +1,5 @@ /** @import { FunctionDeclaration } from 'estree' */ /** @import { ComponentContext } from '../types' */ -import { build_hoisted_params } from '../utils.js'; -import * as b from '#compiler/builders'; /** * @param {FunctionDeclaration} node @@ -10,14 +8,5 @@ import * as b from '#compiler/builders'; export function FunctionDeclaration(node, context) { const state = { ...context.state, in_constructor: false, in_derived: false }; - if (node.metadata?.hoisted === true) { - const params = build_hoisted_params(node, context); - const body = context.visit(node.body, state); - - context.state.hoisted.push(/** @type {FunctionDeclaration} */ ({ ...node, params, body })); - - return b.empty; - } - context.next(state); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index 2fc3a8ed80..0f87baa433 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -7,7 +7,6 @@ import * as b from '#compiler/builders'; import * as assert from '../../../../utils/assert.js'; import { get_rune } from '../../../scope.js'; import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js'; -import { is_hoisted_function } from '../../utils.js'; import { get_value } from './shared/declarations.js'; /** @@ -32,13 +31,6 @@ export function VariableDeclaration(node, context) { rune === '$state.snapshot' || rune === '$host' ) { - if (init != null && is_hoisted_function(init)) { - context.state.hoisted.push( - b.const(declarator.id, /** @type {Expression} */ (context.visit(init))) - ); - - continue; - } declarations.push(/** @type {VariableDeclarator} */ (context.visit(declarator))); continue; } @@ -295,16 +287,6 @@ export function VariableDeclaration(node, context) { const has_props = bindings.some((binding) => binding.kind === 'bindable_prop'); if (!has_state && !has_props) { - const init = declarator.init; - - if (init != null && is_hoisted_function(init)) { - context.state.hoisted.push( - b.const(declarator.id, /** @type {Expression} */ (context.visit(init))) - ); - - continue; - } - declarations.push(/** @type {VariableDeclarator} */ (context.visit(declarator))); continue; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js index d252bd5474..d4d6721960 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js @@ -26,40 +26,12 @@ export function visit_event_attribute(node, context) { let handler = build_event_handler(tag.expression, tag.metadata.expression, context); if (node.metadata.delegated) { - let delegated_assignment; - if (!context.state.events.has(event_name)) { context.state.events.add(event_name); } - // Hoist function if we can, otherwise we leave the function as is - if (node.metadata.delegated.hoisted) { - if (node.metadata.delegated.function === tag.expression) { - const func_name = context.state.scope.root.unique('on_' + event_name); - context.state.hoisted.push(b.var(func_name, handler)); - handler = func_name; - } - - const hoisted_params = /** @type {Expression[]} */ ( - node.metadata.delegated.function.metadata.hoisted_params - ); - - // When we hoist a function we assign an array with the function and all - // hoisted closure params. - if (hoisted_params) { - const args = [handler, ...hoisted_params]; - delegated_assignment = b.array(args); - } else { - delegated_assignment = handler; - } - } else { - delegated_assignment = handler; - } - context.state.init.push( - b.stmt( - b.assignment('=', b.member(context.state.node, '__' + event_name), delegated_assignment) - ) + b.stmt(b.assignment('=', b.member(context.state.node, '__' + event_name), handler)) ); } else { const statement = b.stmt( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js index 691ac0b01e..3677b30814 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js @@ -1,14 +1,11 @@ /** @import { ArrowFunctionExpression, FunctionExpression, Node } from 'estree' */ /** @import { ComponentContext } from '../../types' */ -import { build_hoisted_params } from '../../utils.js'; /** * @param {ArrowFunctionExpression | FunctionExpression} node * @param {ComponentContext} context */ export const visit_function = (node, context) => { - const metadata = node.metadata; - let state = { ...context.state, in_constructor: false, in_derived: false }; if (node.type === 'FunctionExpression') { @@ -16,15 +13,5 @@ export const visit_function = (node, context) => { state.in_constructor = parent.type === 'MethodDefinition' && parent.kind === 'constructor'; } - if (metadata?.hoisted === true) { - const params = build_hoisted_params(node, context); - - return /** @type {FunctionExpression} */ ({ - ...node, - params, - body: context.visit(node.body, state) - }); - } - context.next(state); }; diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index dfc2ab1de1..f61b59f3bd 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -1,36 +1,17 @@ -/** @import { Context } from 'zimmerframe' */ /** @import { TransformState } from './types.js' */ /** @import { AST, Binding, Namespace, ValidatedCompileOptions } from '#compiler' */ /** @import { Node, Expression, CallExpression, MemberExpression } from 'estree' */ import { regex_ends_with_whitespaces, regex_not_whitespace, - regex_starts_with_newline, regex_starts_with_whitespaces } from '../patterns.js'; -import * as b from '#compiler/builders'; import * as e from '../../errors.js'; import { walk } from 'zimmerframe'; import { extract_identifiers } from '../../utils/ast.js'; import check_graph_for_cycles from '../2-analyze/utils/check_graph_for_cycles.js'; import is_reference from 'is-reference'; import { set_scope } from '../scope.js'; -import { dev } from '../../state.js'; - -/** - * @param {Node} node - * @returns {boolean} - */ -export function is_hoisted_function(node) { - if ( - node.type === 'ArrowFunctionExpression' || - node.type === 'FunctionExpression' || - node.type === 'FunctionDeclaration' - ) { - return node.metadata?.hoisted === true; - } - return false; -} /** * Match Svelte 4 behaviour by sorting ConstTag nodes in topological order diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index f4127db359..13188681d2 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -59,7 +59,7 @@ export function create_attribute(name, start, end, value) { name, value, metadata: { - delegated: null, + delegated: false, needs_clsx: false } }; diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index 4e287fd199..074012e03f 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -109,29 +109,3 @@ export interface ComponentAnalysis extends Analysis { */ snippets: Set; } - -declare module 'estree' { - interface ArrowFunctionExpression { - metadata: { - hoisted: boolean; - hoisted_params: Pattern[]; - scope: Scope; - }; - } - - interface FunctionExpression { - metadata: { - hoisted: boolean; - hoisted_params: Pattern[]; - scope: Scope; - }; - } - - interface FunctionDeclaration { - metadata: { - hoisted: boolean; - hoisted_params: Pattern[]; - scope: Scope; - }; - } -} diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 42048c3525..f38706d075 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -5,8 +5,6 @@ import type { VariableDeclaration, VariableDeclarator, Expression, - FunctionDeclaration, - FunctionExpression, Identifier, MemberExpression, Node, @@ -27,13 +25,6 @@ import type { _CSS } from './css'; */ export type Namespace = 'html' | 'svg' | 'mathml'; -export type DelegatedEvent = - | { - hoisted: true; - function: ArrowFunctionExpression | FunctionExpression | FunctionDeclaration; - } - | { hoisted: false }; - export namespace AST { export interface BaseNode { type: string; @@ -531,7 +522,7 @@ export namespace AST { /** @internal */ metadata: { /** May be set if this is an event attribute */ - delegated: null | DelegatedEvent; + delegated: boolean; /** May be `true` if this is a `class` attribute that needs `clsx` */ needs_clsx: boolean; }; diff --git a/packages/svelte/src/compiler/utils/builders.js b/packages/svelte/src/compiler/utils/builders.js index f21b0dc8b4..1a2d5cab5c 100644 --- a/packages/svelte/src/compiler/utils/builders.js +++ b/packages/svelte/src/compiler/utils/builders.js @@ -42,8 +42,7 @@ export function arrow(params, body, async = false) { body, expression: body.type !== 'BlockStatement', generator: false, - async, - metadata: /** @type {any} */ (null) // should not be used by codegen + async }; } @@ -237,8 +236,7 @@ export function function_declaration(id, params, body, async = false) { params, body, generator: false, - async, - metadata: /** @type {any} */ (null) // should not be used by codegen + async }; } @@ -595,8 +593,7 @@ function function_builder(id, params, body, async = false) { params, body, generator: false, - async, - metadata: /** @type {any} */ (null) // should not be used by codegen + async }; } diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index a5f63359c9..55d5d7860a 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -7,7 +7,7 @@ import { add_form_reset_listener, autofocus } from './misc.js'; import * as w from '../../warnings.js'; import { LOADING_ATTR_SYMBOL } from '#client/constants'; import { queue_micro_task } from '../task.js'; -import { is_capture_event, is_delegated, normalize_attribute } from '../../../../utils.js'; +import { is_capture_event, can_delegate_event, normalize_attribute } from '../../../../utils.js'; import { active_effect, active_reaction, @@ -378,7 +378,7 @@ function set_attributes( const opts = {}; const event_handle_key = '$$' + key; let event_name = key.slice(2); - var delegated = is_delegated(event_name); + var delegated = can_delegate_event(event_name); if (is_capture_event(event_name)) { event_name = event_name.slice(0, -7); diff --git a/packages/svelte/src/internal/client/dom/elements/events.js b/packages/svelte/src/internal/client/dom/elements/events.js index 15544d7426..4c64c8364a 100644 --- a/packages/svelte/src/internal/client/dom/elements/events.js +++ b/packages/svelte/src/internal/client/dom/elements/events.js @@ -1,5 +1,5 @@ import { teardown } from '../../reactivity/effects.js'; -import { define_property, is_array } from '../../../shared/utils.js'; +import { define_property } from '../../../shared/utils.js'; import { hydrating } from '../hydration.js'; import { queue_micro_task } from '../task.js'; import { FILENAME } from '../../../../constants.js'; @@ -258,12 +258,7 @@ export function handle_event_propagation(event) { // -> the target could not have been disabled because it emits the event in the first place event.target === current_target) ) { - if (is_array(delegated)) { - var [fn, ...data] = delegated; - fn.apply(current_target, [event, ...data]); - } else { - delegated.call(current_target, event); - } + delegated.call(current_target, event); } } catch (error) { if (throw_error) { diff --git a/packages/svelte/src/utils.js b/packages/svelte/src/utils.js index a54a421418..d63d4ff801 100644 --- a/packages/svelte/src/utils.js +++ b/packages/svelte/src/utils.js @@ -137,7 +137,7 @@ const DELEGATED_EVENTS = [ * Returns `true` if `event_name` is a delegated event * @param {string} event_name */ -export function is_delegated(event_name) { +export function can_delegate_event(event_name) { return DELEGATED_EVENTS.includes(event_name); } diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js index 8134044b16..43d217977e 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js @@ -15,9 +15,9 @@ export default test({ {}, [], { x: 'hello' }, - 'at HTMLButtonElement.on_click', + 'at HTMLButtonElement.Main.button.__click', ['hello'], - 'at HTMLButtonElement.on_click' + 'at HTMLButtonElement.Main.button.__click' ]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-recursive/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-recursive/_config.js index 9d95956e7d..8bf67159f5 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-recursive/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-recursive/_config.js @@ -15,9 +15,9 @@ export default test({ assert.deepEqual(normalise_inspect_logs(logs), [ [], [{}], - 'at HTMLButtonElement.on_click', + 'at HTMLButtonElement.Main.button.__click', [{}, {}], - 'at HTMLButtonElement.on_click' + 'at HTMLButtonElement.Main.button.__click' ]); } }); diff --git a/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/client/index.svelte.js index 9bb45ebf78..52820c1652 100644 --- a/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/await-block-scope/_expected/client/index.svelte.js @@ -1,19 +1,20 @@ import 'svelte/internal/disclose-version'; import * as $ from 'svelte/internal/client'; -function increment(_, counter) { - counter.count += 1; -} - var root = $.from_html(` `, 1); export default function Await_block_scope($$anchor) { let counter = $.proxy({ count: 0 }); const promise = $.derived(() => Promise.resolve(counter)); + + function increment() { + counter.count += 1; + } + var fragment = root(); var button = $.first_child(fragment); - button.__click = [increment, counter]; + button.__click = increment; var text = $.child(button); diff --git a/packages/svelte/tests/snapshot/samples/delegated-locally-declared-shadowed/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/delegated-locally-declared-shadowed/_expected/client/index.svelte.js index 0d95d8d335..ae28419b95 100644 --- a/packages/svelte/tests/snapshot/samples/delegated-locally-declared-shadowed/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/delegated-locally-declared-shadowed/_expected/client/index.svelte.js @@ -2,12 +2,6 @@ import 'svelte/internal/disclose-version'; import 'svelte/internal/flags/legacy'; import * as $ from 'svelte/internal/client'; -var on_click = (e) => { - const index = Number(e.currentTarget.dataset.index); - - console.log(index); -}; - var root_1 = $.from_html(``); export default function Delegated_locally_declared_shadowed($$anchor) { @@ -18,7 +12,13 @@ export default function Delegated_locally_declared_shadowed($$anchor) { var button = root_1(); $.set_attribute(button, 'data-index', index); - button.__click = [on_click]; + + button.__click = (e) => { + const index = Number(e.currentTarget.dataset.index); + + console.log(index); + }; + $.append($$anchor, button); }); diff --git a/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js index b46acee82e..7025c788be 100644 --- a/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/nullish-coallescence-omittance/_expected/client/index.svelte.js @@ -1,7 +1,6 @@ import 'svelte/internal/disclose-version'; import * as $ from 'svelte/internal/client'; -var on_click = (_, count) => $.update(count); var root = $.from_html(`

`, 1); export default function Nullish_coallescence_omittance($$anchor) { @@ -18,7 +17,7 @@ export default function Nullish_coallescence_omittance($$anchor) { var button = $.sibling(b, 2); - button.__click = [on_click, count]; + button.__click = () => $.update(count); var text = $.child(button); diff --git a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js index c446b3d3ef..30691231f4 100644 --- a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js @@ -1,18 +1,19 @@ import 'svelte/internal/disclose-version'; import * as $ from 'svelte/internal/client'; -function reset(_, str, tpl) { - $.set(str, ''); - $.set(str, ``); - $.set(tpl, ''); - $.set(tpl, ``); -} - var root = $.from_html(` `, 1); export default function State_proxy_literal($$anchor) { let str = $.state(''); let tpl = $.state(``); + + function reset() { + $.set(str, ''); + $.set(str, ``); + $.set(tpl, ''); + $.set(tpl, ``); + } + var fragment = root(); var input = $.first_child(fragment); @@ -24,7 +25,7 @@ export default function State_proxy_literal($$anchor) { var button = $.sibling(input_1, 2); - button.__click = [reset, str, tpl]; + button.__click = reset; $.bind_value(input, () => $.get(str), ($$value) => $.set(str, $$value)); $.bind_value(input_1, () => $.get(tpl), ($$value) => $.set(tpl, $$value)); $.append($$anchor, fragment);