diff --git a/documentation/docs/07-misc/01-best-practices.md b/documentation/docs/07-misc/01-best-practices.md index ac275e4d9a..66f7da2613 100644 --- a/documentation/docs/07-misc/01-best-practices.md +++ b/documentation/docs/07-misc/01-best-practices.md @@ -1,5 +1,6 @@ --- title: Best practices +skill: true name: svelte-core-bestpractices description: Guidance on writing fast, robust, modern Svelte code. Load this skill whenever in a Svelte project and asked to write/edit or analyze a Svelte component or module. Covers reactivity, event handling, styling, integration with libraries and more. --- diff --git a/documentation/docs/07-misc/07-v5-migration-guide.md b/documentation/docs/07-misc/07-v5-migration-guide.md index 580dbec6d4..9b1f2dec63 100644 --- a/documentation/docs/07-misc/07-v5-migration-guide.md +++ b/documentation/docs/07-misc/07-v5-migration-guide.md @@ -324,7 +324,7 @@ When spreading props, local event handlers must go _after_ the spread, or they r > > It was always possible to use component callback props, but because you had to listen to DOM events using `on:`, it made sense to use `createEventDispatcher` for component events due to syntactical consistency. Now that we have event attributes (`onclick`), it's the other way around: Callback props are now the more sensible thing to do. > -> The removal of event modifiers is arguably one of the changes that seems like a step back for those who've liked the shorthand syntax of event modifiers. Given that they are not used that frequently, we traded a smaller surface area for more explicitness. Modifiers also were inconsistent, because most of them were only useable on DOM elements. +> The removal of event modifiers is arguably one of the changes that seems like a step back for those who've liked the shorthand syntax of event modifiers. Given that they are not used that frequently, we traded a smaller surface area for more explicitness. Modifiers also were inconsistent, because most of them were only usable on DOM elements. > > Multiple listeners for the same event are also no longer possible, but it was something of an anti-pattern anyway, since it impedes readability: if there are many attributes, it becomes harder to spot that there are two handlers unless they are right next to each other. It also implies that the two handlers are independent, when in fact something like `event.stopImmediatePropagation()` inside `one` would prevent `two` from being called. > diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index 5e1a5cc6f1..f09032294a 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,25 @@ # svelte +## 5.53.9 + +### Patch Changes + +- fix: better `bind:this` cleanup timing ([#17885](https://github.com/sveltejs/svelte/pull/17885)) + +## 5.53.8 + +### Patch Changes + +- fix: `{@html}` no longer duplicates content inside `contenteditable` elements ([#17853](https://github.com/sveltejs/svelte/pull/17853)) + +- fix: don't access inert block effects ([#17882](https://github.com/sveltejs/svelte/pull/17882)) + +- fix: handle asnyc updates within pending boundary ([#17873](https://github.com/sveltejs/svelte/pull/17873)) + +- perf: avoid re-traversing the effect tree after `$:` assignments ([#17848](https://github.com/sveltejs/svelte/pull/17848)) + +- chore: simplify scheduling logic ([#17805](https://github.com/sveltejs/svelte/pull/17805)) + ## 5.53.7 ### Patch Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 5f5b6c3615..17fc4335d5 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -2,7 +2,7 @@ "name": "svelte", "description": "Cybernetically enhanced web apps", "license": "MIT", - "version": "5.53.7", + "version": "5.53.9", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js index 2706cf7f0a..6c8b7c0354 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js @@ -9,7 +9,11 @@ import { build_expression } from './shared/utils.js'; * @param {ComponentContext} context */ export function HtmlTag(node, context) { - context.state.template.push_comment(); + const is_controlled = node.metadata.is_controlled; + + if (!is_controlled) { + context.state.template.push_comment(); + } const has_await = node.metadata.expression.has_await; const has_blockers = node.metadata.expression.has_blockers(); @@ -17,14 +21,17 @@ export function HtmlTag(node, context) { const expression = build_expression(context, node.expression, node.metadata.expression); const html = has_await ? b.call('$.get', b.id('$$html')) : expression; - const is_svg = context.state.metadata.namespace === 'svg'; - const is_mathml = context.state.metadata.namespace === 'mathml'; + // When is_controlled, the parent node already provides the correct namespace, + // so is_svg/is_mathml are only needed for the non-controlled path's wrapper element + const is_svg = !is_controlled && context.state.metadata.namespace === 'svg'; + const is_mathml = !is_controlled && context.state.metadata.namespace === 'mathml'; const statement = b.stmt( b.call( '$.html', context.state.node, b.thunk(html), + is_controlled && b.true, is_svg && b.true, is_mathml && b.true, is_ignored(node, 'hydration_html_changed') && b.true diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js index 59b93f24ef..bd3e708662 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js @@ -109,6 +109,8 @@ export function process_children(nodes, initial, is_element, context) { !node.metadata.expression.is_async() ) { node.metadata.is_controlled = true; + } else if (node.type === 'HtmlTag' && nodes.length === 1 && is_element) { + node.metadata.is_controlled = true; } else { const id = flush_node( false, diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index d44a31349a..3c1e3e772c 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -133,6 +133,8 @@ export namespace AST { /** @internal */ metadata: { expression: ExpressionMetadata; + /** If `true`, the `{@html}` block is the only child of its parent element and can use `parent.innerHTML` directly */ + is_controlled?: boolean; }; } diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 3525539f1d..df96f4899b 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -29,6 +29,8 @@ export const INERT = 1 << 13; export const DESTROYED = 1 << 14; /** Set once a reaction has run for the first time */ export const REACTION_RAN = 1 << 15; +/** Effect is in the process of getting destroyed. Can be observed in child teardown functions */ +export const DESTROYING = 1 << 25; // Flags exclusive to effects /** diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js index 36c735272c..0baef5c63e 100644 --- a/packages/svelte/src/internal/client/context.js +++ b/packages/svelte/src/internal/client/context.js @@ -5,7 +5,7 @@ import { active_effect, active_reaction } from './runtime.js'; import { create_user_effect } from './reactivity/effects.js'; import { async_mode_flag, legacy_mode_flag } from '../flags/index.js'; import { FILENAME } from '../../constants.js'; -import { BRANCH_EFFECT, REACTION_RAN } from './constants.js'; +import { BRANCH_EFFECT } from './constants.js'; /** @type {ComponentContext | null} */ export let component_context = null; @@ -182,6 +182,7 @@ export function push(props, runes = false, fn) { e: null, s: props, x: null, + r: /** @type {Effect} */ (active_effect), l: legacy_mode_flag && !runes ? { s: null, u: null, $: [] } : null }; diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 09a3ec5ca4..d6430547b5 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -45,7 +45,14 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) { var branches = new BranchManager(node); block(() => { + var batch = /** @type {Batch} */ (current_batch); + + // we null out `current_batch` because otherwise `save(...)` will incorrectly restore it — + // the batch will already have been committed by the time it resolves + batch.deactivate(); var input = get_input(); + batch.activate(); + var destroyed = false; /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 429a2eb293..b38a3131ca 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -35,7 +35,7 @@ import { queue_micro_task } from '../task.js'; import * as e from '../../errors.js'; import * as w from '../../warnings.js'; import { DEV } from 'esm-env'; -import { Batch, schedule_effect } from '../../reactivity/batch.js'; +import { Batch, current_batch, schedule_effect } from '../../reactivity/batch.js'; import { internal_set, source } from '../../reactivity/sources.js'; import { tag } from '../../dev/tracing.js'; import { createSubscriber } from '../../../../reactivity/create-subscriber.js'; @@ -218,6 +218,8 @@ export class Boundary { this.is_pending = true; this.#pending_effect = branch(() => pending(this.#anchor)); + var batch = /** @type {Batch} */ (current_batch); + queue_micro_task(() => { var fragment = (this.#offscreen_fragment = document.createDocumentFragment()); var anchor = create_text(); @@ -225,7 +227,6 @@ export class Boundary { fragment.append(anchor); this.#main_effect = this.#run(() => { - Batch.ensure(); return branch(() => this.#children(anchor)); }); @@ -237,12 +238,14 @@ export class Boundary { this.#pending_effect = null; }); - this.#resolve(); + this.#resolve(batch); } }); } #render() { + var batch = /** @type {Batch} */ (current_batch); + try { this.is_pending = this.has_pending_snippet(); this.#pending_count = 0; @@ -259,14 +262,17 @@ export class Boundary { const pending = /** @type {(anchor: Node) => void} */ (this.#props.pending); this.#pending_effect = branch(() => pending(this.#anchor)); } else { - this.#resolve(); + this.#resolve(batch); } } catch (error) { this.error(error); } } - #resolve() { + /** + * @param {Batch} batch + */ + #resolve(batch) { this.is_pending = false; // any effects that were previously deferred should be rescheduled — @@ -274,12 +280,12 @@ export class Boundary { // same update that brought us here) the effects will be flushed for (const e of this.#dirty_effects) { set_signal_status(e, DIRTY); - schedule_effect(e); + batch.schedule(e); } for (const e of this.#maybe_dirty_effects) { set_signal_status(e, MAYBE_DIRTY); - schedule_effect(e); + batch.schedule(e); } this.#dirty_effects.clear(); @@ -320,6 +326,7 @@ export class Boundary { set_component_context(this.#effect.ctx); try { + Batch.ensure(); return fn(); } catch (e) { handle_error(e); @@ -335,11 +342,12 @@ export class Boundary { * Updates the pending count associated with the currently visible pending snippet, * if any, such that we can replace the snippet with content once work is done * @param {1 | -1} d + * @param {Batch} batch */ - #update_pending_count(d) { + #update_pending_count(d, batch) { if (!this.has_pending_snippet()) { if (this.parent) { - this.parent.#update_pending_count(d); + this.parent.#update_pending_count(d, batch); } // if there's no parent, we're in a scope with no pending snippet @@ -349,7 +357,7 @@ export class Boundary { this.#pending_count += d; if (this.#pending_count === 0) { - this.#resolve(); + this.#resolve(batch); if (this.#pending_effect) { pause_effect(this.#pending_effect, () => { @@ -369,9 +377,10 @@ export class Boundary { * and controls when the current `pending` snippet (if any) is removed. * Do not call from inside the class * @param {1 | -1} d + * @param {Batch} batch */ - update_pending_count(d) { - this.#update_pending_count(d); + update_pending_count(d, batch) { + this.#update_pending_count(d, batch); this.#local_pending_count += d; @@ -445,9 +454,6 @@ export class Boundary { } this.#run(() => { - // If the failure happened while flushing effects, current_batch can be null - Batch.ensure(); - this.#render(); }); }; @@ -464,8 +470,6 @@ export class Boundary { if (failed) { this.#failed_effect = this.#run(() => { - Batch.ensure(); - try { return branch(() => { // errors in `failed` snippets cause the boundary to error again diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index b49af0859f..c3b5d23915 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 { INERT } from '#client/constants'; import { Batch, current_batch } from '../../reactivity/batch.js'; import { branch, @@ -88,7 +87,7 @@ export class BranchManager { // effect is currently offscreen. put it in the DOM var offscreen = this.#offscreen.get(key); - if (offscreen && (offscreen.effect.f & INERT) === 0) { + if (offscreen) { this.#onscreen.set(key, offscreen.effect); this.#offscreen.delete(key); @@ -125,9 +124,6 @@ export class BranchManager { // or those that are already outroing (else the transition is aborted and the effect destroyed right away) if (k === key || this.#outroing.has(k)) continue; - // don't destroy branches that are inside outroing blocks - if ((effect.f & INERT) !== 0) continue; - const on_destroy = () => { const keys = Array.from(this.#batches.values()); diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index af66a04534..ffe947eb16 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -42,17 +42,33 @@ function check_hash(element, server_hash, value) { /** * @param {Element | Text | Comment} node * @param {() => string | TrustedHTML} get_value + * @param {boolean} [is_controlled] * @param {boolean} [svg] * @param {boolean} [mathml] * @param {boolean} [skip_warning] * @returns {void} */ -export function html(node, get_value, svg = false, mathml = false, skip_warning = false) { +export function html( + node, + get_value, + is_controlled = false, + svg = false, + mathml = false, + skip_warning = false +) { var anchor = node; /** @type {string | TrustedHTML} */ var value = ''; + if (is_controlled) { + var parent_node = /** @type {Element} */ (node); + + if (hydrating) { + anchor = set_hydrate_node(get_first_child(parent_node)); + } + } + template_effect(() => { var effect = /** @type {Effect} */ (active_effect); @@ -61,6 +77,22 @@ export function html(node, get_value, svg = false, mathml = false, skip_warning return; } + if (is_controlled && !hydrating) { + // When @html is the only child, use innerHTML directly. + // This also handles contenteditable, where the user may delete the anchor comment. + effect.nodes = null; + parent_node.innerHTML = /** @type {string} */ (value); + + if (value !== '') { + assign_nodes( + /** @type {TemplateNode} */ (get_first_child(parent_node)), + /** @type {TemplateNode} */ (parent_node.lastChild) + ); + } + + return; + } + if (effect.nodes !== null) { remove_effect_dom(effect.nodes.start, /** @type {TemplateNode} */ (effect.nodes.end)); effect.nodes = null; diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js index 23ad6f5cdc..55e61c3774 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -9,6 +9,7 @@ import { hydrating } from '../../hydration.js'; import { tick, untrack } from '../../../runtime.js'; import { is_runes } from '../../../context.js'; import { current_batch, previous_batch } from '../../../reactivity/batch.js'; +import { async_mode_flag } from '../../../../flags/index.js'; /** * @param {HTMLInputElement} input @@ -87,8 +88,9 @@ export function bind_value(input, get, set = get) { var value = get(); if (input === document.activeElement) { - // we need both, because in non-async mode, render effects run before previous_batch is set - var batch = /** @type {Batch} */ (previous_batch ?? current_batch); + // In sync mode render effects are executed during tree traversal -> needs current_batch + // In async mode render effects are flushed once batch resolved, at which point current_batch is null -> needs previous_batch + var batch = /** @type {Batch} */ (async_mode_flag ? previous_batch : current_batch); // Never rewrite the contents of a focused input. We can get here if, for example, // an update is deferred because of async work depending on the input: diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index 46e8f524f8..21be75ba61 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -4,6 +4,7 @@ import { is } from '../../../proxy.js'; import { is_array } from '../../../../shared/utils.js'; import * as w from '../../../warnings.js'; import { Batch, current_batch, previous_batch } from '../../../reactivity/batch.js'; +import { async_mode_flag } from '../../../../flags/index.js'; /** * Selects the correct option(s) (depending on whether this is a multiple select) @@ -115,8 +116,9 @@ export function bind_select_value(select, get, set = get) { var value = get(); if (select === document.activeElement) { - // we need both, because in non-async mode, render effects run before previous_batch is set - var batch = /** @type {Batch} */ (previous_batch ?? current_batch); + // In sync mode render effects are executed during tree traversal -> needs current_batch + // In async mode render effects are flushed once batch resolved, at which point current_batch is null -> needs previous_batch + var batch = /** @type {Batch} */ (async_mode_flag ? previous_batch : current_batch); // Don't update the