From 5e9b29c351294e84d8e652ac871fbe3e7d620351 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 28 Jan 2025 15:57:26 -0500 Subject: [PATCH 1/5] chore: move context code into new module (#15132) --- packages/svelte/src/index-client.js | 15 +- .../svelte/src/internal/client/context.js | 214 +++++++++++++++++ .../svelte/src/internal/client/dev/legacy.js | 2 +- .../src/internal/client/dev/ownership.js | 2 +- .../src/internal/client/dom/blocks/await.js | 13 +- .../internal/client/dom/blocks/boundary.js | 3 +- .../src/internal/client/dom/blocks/html.js | 2 +- .../src/internal/client/dom/blocks/snippet.js | 2 +- .../client/dom/blocks/svelte-element.js | 3 +- .../internal/client/dom/legacy/lifecycle.js | 3 +- packages/svelte/src/internal/client/index.js | 9 +- packages/svelte/src/internal/client/proxy.js | 5 +- .../internal/client/reactivity/deriveds.js | 4 +- .../src/internal/client/reactivity/effects.js | 3 +- .../src/internal/client/reactivity/sources.js | 2 +- packages/svelte/src/internal/client/render.js | 3 +- .../svelte/src/internal/client/runtime.js | 220 +----------------- .../svelte/src/internal/client/validate.js | 2 +- packages/svelte/src/legacy/legacy-client.js | 10 +- packages/svelte/tests/signals/test.ts | 5 +- packages/svelte/types/index.d.ts | 56 ++--- 21 files changed, 288 insertions(+), 290 deletions(-) create mode 100644 packages/svelte/src/internal/client/context.js diff --git a/packages/svelte/src/index-client.js b/packages/svelte/src/index-client.js index 587d766233..bd2b353395 100644 --- a/packages/svelte/src/index-client.js +++ b/packages/svelte/src/index-client.js @@ -1,12 +1,13 @@ /** @import { ComponentContext, ComponentContextLegacy } from '#client' */ /** @import { EventDispatcher } from './index.js' */ /** @import { NotFunction } from './internal/types.js' */ -import { component_context, flush_sync, untrack } from './internal/client/runtime.js'; +import { flush_sync, untrack } from './internal/client/runtime.js'; import { is_array } from './internal/shared/utils.js'; import { user_effect } from './internal/client/index.js'; import * as e from './internal/client/errors.js'; import { lifecycle_outside_component } from './internal/shared/errors.js'; import { legacy_mode_flag } from './internal/flags/index.js'; +import { component_context } from './internal/client/context.js'; /** * The `onMount` function schedules a callback to run as soon as the component has been mounted to the DOM. @@ -179,15 +180,7 @@ export function flushSync(fn) { flush_sync(fn); } +export { getContext, getAllContexts, hasContext, setContext } from './internal/client/context.js'; export { hydrate, mount, unmount } from './internal/client/render.js'; - -export { - getContext, - getAllContexts, - hasContext, - setContext, - tick, - untrack -} from './internal/client/runtime.js'; - +export { tick, untrack } from './internal/client/runtime.js'; export { createRawSnippet } from './internal/client/dom/blocks/snippet.js'; diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js new file mode 100644 index 0000000000..1d7f0747cd --- /dev/null +++ b/packages/svelte/src/internal/client/context.js @@ -0,0 +1,214 @@ +/** @import { ComponentContext } from '#client' */ + +import { DEV } from 'esm-env'; +import { add_owner } from './dev/ownership.js'; +import { lifecycle_outside_component } from '../shared/errors.js'; +import { source } from './reactivity/sources.js'; +import { + active_effect, + active_reaction, + set_active_effect, + set_active_reaction +} from './runtime.js'; +import { effect } from './reactivity/effects.js'; +import { legacy_mode_flag } from '../flags/index.js'; + +/** @type {ComponentContext | null} */ +export let component_context = null; + +/** @param {ComponentContext | null} context */ +export function set_component_context(context) { + component_context = context; +} + +/** + * The current component function. Different from current component context: + * ```html + * + * + * + * + * ``` + * @type {ComponentContext['function']} + */ +export let dev_current_component_function = null; + +/** @param {ComponentContext['function']} fn */ +export function set_dev_current_component_function(fn) { + dev_current_component_function = fn; +} + +/** + * Retrieves the context that belongs to the closest parent component with the specified `key`. + * Must be called during component initialisation. + * + * @template T + * @param {any} key + * @returns {T} + */ +export function getContext(key) { + const context_map = get_or_init_context_map('getContext'); + const result = /** @type {T} */ (context_map.get(key)); + + if (DEV) { + const fn = /** @type {ComponentContext} */ (component_context).function; + if (fn) { + add_owner(result, fn, true); + } + } + + return result; +} + +/** + * Associates an arbitrary `context` object with the current component and the specified `key` + * and returns that object. The context is then available to children of the component + * (including slotted content) with `getContext`. + * + * Like lifecycle functions, this must be called during component initialisation. + * + * @template T + * @param {any} key + * @param {T} context + * @returns {T} + */ +export function setContext(key, context) { + const context_map = get_or_init_context_map('setContext'); + context_map.set(key, context); + return context; +} + +/** + * Checks whether a given `key` has been set in the context of a parent component. + * Must be called during component initialisation. + * + * @param {any} key + * @returns {boolean} + */ +export function hasContext(key) { + const context_map = get_or_init_context_map('hasContext'); + return context_map.has(key); +} + +/** + * Retrieves the whole context map that belongs to the closest parent component. + * Must be called during component initialisation. Useful, for example, if you + * programmatically create a component and want to pass the existing context to it. + * + * @template {Map} [T=Map] + * @returns {T} + */ +export function getAllContexts() { + const context_map = get_or_init_context_map('getAllContexts'); + + if (DEV) { + const fn = component_context?.function; + if (fn) { + for (const value of context_map.values()) { + add_owner(value, fn, true); + } + } + } + + return /** @type {T} */ (context_map); +} + +/** + * @param {Record} props + * @param {any} runes + * @param {Function} [fn] + * @returns {void} + */ +export function push(props, runes = false, fn) { + component_context = { + p: component_context, + c: null, + e: null, + m: false, + s: props, + x: null, + l: null + }; + + if (legacy_mode_flag && !runes) { + component_context.l = { + s: null, + u: null, + r1: [], + r2: source(false) + }; + } + + if (DEV) { + // component function + component_context.function = fn; + dev_current_component_function = fn; + } +} + +/** + * @template {Record} T + * @param {T} [component] + * @returns {T} + */ +export function pop(component) { + const context_stack_item = component_context; + if (context_stack_item !== null) { + if (component !== undefined) { + context_stack_item.x = component; + } + const component_effects = context_stack_item.e; + if (component_effects !== null) { + var previous_effect = active_effect; + var previous_reaction = active_reaction; + context_stack_item.e = null; + try { + for (var i = 0; i < component_effects.length; i++) { + var component_effect = component_effects[i]; + set_active_effect(component_effect.effect); + set_active_reaction(component_effect.reaction); + effect(component_effect.fn); + } + } finally { + set_active_effect(previous_effect); + set_active_reaction(previous_reaction); + } + } + component_context = context_stack_item.p; + if (DEV) { + dev_current_component_function = context_stack_item.p?.function ?? null; + } + context_stack_item.m = true; + } + // Micro-optimization: Don't set .a above to the empty object + // so it can be garbage-collected when the return here is unused + return component || /** @type {T} */ ({}); +} + +/** + * @param {string} name + * @returns {Map} + */ +function get_or_init_context_map(name) { + if (component_context === null) { + lifecycle_outside_component(name); + } + + return (component_context.c ??= new Map(get_parent_context(component_context) || undefined)); +} + +/** + * @param {ComponentContext} component_context + * @returns {Map | null} + */ +function get_parent_context(component_context) { + let parent = component_context.p; + while (parent !== null) { + const context_map = parent.c; + if (context_map !== null) { + return context_map; + } + parent = parent.p; + } + return null; +} diff --git a/packages/svelte/src/internal/client/dev/legacy.js b/packages/svelte/src/internal/client/dev/legacy.js index a1d2fc8823..138213c551 100644 --- a/packages/svelte/src/internal/client/dev/legacy.js +++ b/packages/svelte/src/internal/client/dev/legacy.js @@ -1,5 +1,5 @@ import * as e from '../errors.js'; -import { component_context } from '../runtime.js'; +import { component_context } from '../context.js'; import { FILENAME } from '../../../constants.js'; import { get_component } from './ownership.js'; diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index d113d9ae90..70cfbb47f3 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -3,7 +3,7 @@ import { STATE_SYMBOL_METADATA } from '../constants.js'; import { render_effect, user_pre_effect } from '../reactivity/effects.js'; -import { dev_current_component_function } from '../runtime.js'; +import { dev_current_component_function } from '../context.js'; import { get_prototype_of } from '../../shared/utils.js'; import * as w from '../warnings.js'; import { FILENAME } from '../../../constants.js'; diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 62b2e4dd0c..633a6e5bab 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -3,18 +3,15 @@ import { DEV } from 'esm-env'; import { is_promise } from '../../../shared/utils.js'; import { block, branch, pause_effect, resume_effect } from '../../reactivity/effects.js'; import { internal_set, mutable_source, source } from '../../reactivity/sources.js'; +import { flush_sync, is_runes, set_active_effect, set_active_reaction } from '../../runtime.js'; +import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; +import { queue_micro_task } from '../task.js'; +import { UNINITIALIZED } from '../../../../constants.js'; import { component_context, - flush_sync, - is_runes, - set_active_effect, - set_active_reaction, set_component_context, set_dev_current_component_function -} from '../../runtime.js'; -import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; -import { queue_micro_task } from '../task.js'; -import { UNINITIALIZED } from '../../../../constants.js'; +} from '../../context.js'; const PENDING = 0; const THEN = 1; diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 7f4f000dce..c1ca7a9600 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -1,15 +1,14 @@ /** @import { Effect, TemplateNode, } from '#client' */ import { BOUNDARY_EFFECT, EFFECT_TRANSPARENT } from '../../constants.js'; +import { component_context, set_component_context } from '../../context.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; import { active_effect, active_reaction, - component_context, handle_error, set_active_effect, set_active_reaction, - set_component_context, reset_is_throwing_error } from '../../runtime.js'; import { diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index 04ab0aee87..b3fc5a9c72 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -7,7 +7,7 @@ import { assign_nodes } from '../template.js'; import * as w from '../../warnings.js'; import { hash, sanitize_location } from '../../../../utils.js'; import { DEV } from 'esm-env'; -import { dev_current_component_function } from '../../runtime.js'; +import { dev_current_component_function } from '../../context.js'; import { get_first_child, get_next_sibling } from '../operations.js'; /** diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index cec57f83b7..b916a02ce5 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -6,7 +6,7 @@ import { branch, block, destroy_effect, teardown } from '../../reactivity/effect import { dev_current_component_function, set_dev_current_component_function -} from '../../runtime.js'; +} from '../../context.js'; import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; import { create_fragment_from_html } from '../reconciler.js'; import { assign_nodes } from '../template.js'; diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index 35d2f223ae..18641300e5 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -17,7 +17,8 @@ import { } from '../../reactivity/effects.js'; import { set_should_intro } from '../../render.js'; import { current_each_item, set_current_each_item } from './each.js'; -import { component_context, active_effect } from '../../runtime.js'; +import { active_effect } from '../../runtime.js'; +import { component_context } from '../../context.js'; import { DEV } from 'esm-env'; import { EFFECT_TRANSPARENT } from '../../constants.js'; import { assign_nodes } from '../template.js'; diff --git a/packages/svelte/src/internal/client/dom/legacy/lifecycle.js b/packages/svelte/src/internal/client/dom/legacy/lifecycle.js index 5ffbacc670..a564712f04 100644 --- a/packages/svelte/src/internal/client/dom/legacy/lifecycle.js +++ b/packages/svelte/src/internal/client/dom/legacy/lifecycle.js @@ -1,8 +1,9 @@ /** @import { ComponentContextLegacy } from '#client' */ import { run, run_all } from '../../../shared/utils.js'; +import { component_context } from '../../context.js'; import { derived } from '../../reactivity/deriveds.js'; import { user_pre_effect, user_effect } from '../../reactivity/effects.js'; -import { component_context, deep_read_state, get, untrack } from '../../runtime.js'; +import { deep_read_state, get, untrack } from '../../runtime.js'; /** * Legacy-mode only: Call `onMount` callbacks and set up `beforeUpdate`/`afterUpdate` effects diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 2bf58c51f7..877f3c59ee 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -1,4 +1,5 @@ export { FILENAME, HMR, NAMESPACE_SVG } from '../../constants.js'; +export { push, pop } from './context.js'; export { assign, assign_and, assign_or, assign_nullish } from './dev/assign.js'; export { cleanup_styles } from './dev/css.js'; export { add_locations } from './dev/elements.js'; @@ -141,14 +142,8 @@ export { update, update_pre, exclude_from_object, - pop, - push, deep_read, - deep_read_state, - getAllContexts, - getContext, - setContext, - hasContext + deep_read_state } from './runtime.js'; export { validate_binding, validate_each_keys } from './validate.js'; export { raf } from './timing.js'; diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 6cbd6394df..4c262880f1 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -1,6 +1,7 @@ -/** @import { ProxyMetadata, ProxyStateObject, Source } from '#client' */ +/** @import { ProxyMetadata, Source } from '#client' */ import { DEV } from 'esm-env'; -import { get, component_context, active_effect } from './runtime.js'; +import { get, active_effect } from './runtime.js'; +import { component_context } from './context.js'; import { array_prototype, get_descriptor, diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index da5a75eb6e..e86963408c 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -17,8 +17,7 @@ import { skip_reaction, update_reaction, increment_write_version, - set_active_effect, - component_context + set_active_effect } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; @@ -26,6 +25,7 @@ import { destroy_effect } from './effects.js'; import { inspect_effects, set_inspect_effects } from './sources.js'; import { get_stack } from '../dev/tracing.js'; import { tracing_mode_flag } from '../../flags/index.js'; +import { component_context } from '../context.js'; /** * @template V diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 1faf9a47a0..8a7cffecd6 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -1,10 +1,8 @@ /** @import { ComponentContext, ComponentContextLegacy, Derived, Effect, TemplateNode, TransitionManager } from '#client' */ import { check_dirtiness, - component_context, active_effect, active_reaction, - dev_current_component_function, update_effect, get, is_destroying_effect, @@ -45,6 +43,7 @@ import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; import { get_next_sibling } from '../dom/operations.js'; import { derived, destroy_derived } from './deriveds.js'; +import { component_context, dev_current_component_function } from '../context.js'; /** * @param {'$effect' | '$effect.pre' | '$inspect'} rune diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index c2448c9ee5..b3e66a3336 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -1,7 +1,6 @@ /** @import { Derived, Effect, Reaction, Source, Value } from '#client' */ import { DEV } from 'esm-env'; import { - component_context, active_reaction, active_effect, untracked_writes, @@ -35,6 +34,7 @@ import { import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { get_stack } from '../dev/tracing.js'; +import { component_context } from '../context.js'; export let inspect_effects = new Set(); diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 767b230131..bc74047f64 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -9,7 +9,8 @@ import { init_operations } from './dom/operations.js'; import { HYDRATION_END, HYDRATION_ERROR, HYDRATION_START } from '../../constants.js'; -import { push, pop, component_context, active_effect } from './runtime.js'; +import { active_effect } from './runtime.js'; +import { push, pop, component_context } from './context.js'; import { component_root, branch } from './reactivity/effects.js'; import { hydrate_next, diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a002665b51..859925186f 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -5,7 +5,6 @@ import { destroy_block_effect_children, destroy_effect_children, destroy_effect_deriveds, - effect, execute_effect_teardown, unlink_effect } from './reactivity/effects.js'; @@ -28,14 +27,18 @@ import { BOUNDARY_EFFECT } from './constants.js'; import { flush_tasks } from './dom/task.js'; -import { add_owner } from './dev/ownership.js'; -import { internal_set, set, source } from './reactivity/sources.js'; +import { internal_set, set } from './reactivity/sources.js'; import { destroy_derived, execute_derived, update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; -import { lifecycle_outside_component } from '../shared/errors.js'; import { FILENAME } from '../../constants.js'; import { legacy_mode_flag, tracing_mode_flag } from '../flags/index.js'; import { tracing_expressions, get_stack } from './dev/tracing.js'; +import { + component_context, + dev_current_component_function, + set_component_context, + set_dev_current_component_function +} from './context.js'; const FLUSH_MICROTASK = 0; const FLUSH_SYNC = 1; @@ -150,32 +153,6 @@ export function set_captured_signals(value) { captured_signals = value; } -// Handling runtime component context -/** @type {ComponentContext | null} */ -export let component_context = null; - -/** @param {ComponentContext | null} context */ -export function set_component_context(context) { - component_context = context; -} - -/** - * The current component function. Different from current component context: - * ```html - * - * - * - * - * ``` - * @type {ComponentContext['function']} - */ -export let dev_current_component_function = null; - -/** @param {ComponentContext['function']} fn */ -export function set_dev_current_component_function(fn) { - dev_current_component_function = fn; -} - export function increment_write_version() { return ++write_version; } @@ -434,7 +411,7 @@ export function update_reaction(reaction) { active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null; skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0; derived_sources = null; - component_context = reaction.ctx; + set_component_context(reaction.ctx); untracking = false; read_version++; @@ -498,7 +475,7 @@ export function update_reaction(reaction) { active_reaction = previous_reaction; skip_reaction = previous_skip_reaction; derived_sources = prev_derived_sources; - component_context = previous_component_context; + set_component_context(previous_component_context); untracking = previous_untracking; } } @@ -578,7 +555,7 @@ export function update_effect(effect) { if (DEV) { var previous_component_fn = dev_current_component_function; - dev_current_component_function = effect.component_function; + set_dev_current_component_function(effect.component_function); } try { @@ -620,7 +597,7 @@ export function update_effect(effect) { active_effect = previous_effect; if (DEV) { - dev_current_component_function = previous_component_fn; + set_dev_current_component_function(previous_component_fn); } } } @@ -1114,109 +1091,6 @@ export function set_signal_status(signal, status) { signal.f = (signal.f & STATUS_MASK) | status; } -/** - * Retrieves the context that belongs to the closest parent component with the specified `key`. - * Must be called during component initialisation. - * - * @template T - * @param {any} key - * @returns {T} - */ -export function getContext(key) { - const context_map = get_or_init_context_map('getContext'); - const result = /** @type {T} */ (context_map.get(key)); - - if (DEV) { - const fn = /** @type {ComponentContext} */ (component_context).function; - if (fn) { - add_owner(result, fn, true); - } - } - - return result; -} - -/** - * Associates an arbitrary `context` object with the current component and the specified `key` - * and returns that object. The context is then available to children of the component - * (including slotted content) with `getContext`. - * - * Like lifecycle functions, this must be called during component initialisation. - * - * @template T - * @param {any} key - * @param {T} context - * @returns {T} - */ -export function setContext(key, context) { - const context_map = get_or_init_context_map('setContext'); - context_map.set(key, context); - return context; -} - -/** - * Checks whether a given `key` has been set in the context of a parent component. - * Must be called during component initialisation. - * - * @param {any} key - * @returns {boolean} - */ -export function hasContext(key) { - const context_map = get_or_init_context_map('hasContext'); - return context_map.has(key); -} - -/** - * Retrieves the whole context map that belongs to the closest parent component. - * Must be called during component initialisation. Useful, for example, if you - * programmatically create a component and want to pass the existing context to it. - * - * @template {Map} [T=Map] - * @returns {T} - */ -export function getAllContexts() { - const context_map = get_or_init_context_map('getAllContexts'); - - if (DEV) { - const fn = component_context?.function; - if (fn) { - for (const value of context_map.values()) { - add_owner(value, fn, true); - } - } - } - - return /** @type {T} */ (context_map); -} - -/** - * @param {string} name - * @returns {Map} - */ -function get_or_init_context_map(name) { - if (component_context === null) { - lifecycle_outside_component(name); - } - - return (component_context.c ??= new Map(get_parent_context(component_context) || undefined)); -} - -/** - * @param {ComponentContext} component_context - * @returns {Map | null} - */ -function get_parent_context(component_context) { - let parent = component_context.p; - while (parent !== null) { - const context_map = parent.c; - if (context_map !== null) { - return context_map; - } - parent = parent.p; - } - return null; -} - /** * @template {number | bigint} T * @param {Value} signal @@ -1264,78 +1138,6 @@ export function exclude_from_object(obj, keys) { return result; } -/** - * @param {Record} props - * @param {any} runes - * @param {Function} [fn] - * @returns {void} - */ -export function push(props, runes = false, fn) { - component_context = { - p: component_context, - c: null, - e: null, - m: false, - s: props, - x: null, - l: null - }; - - if (legacy_mode_flag && !runes) { - component_context.l = { - s: null, - u: null, - r1: [], - r2: source(false) - }; - } - - if (DEV) { - // component function - component_context.function = fn; - dev_current_component_function = fn; - } -} - -/** - * @template {Record} T - * @param {T} [component] - * @returns {T} - */ -export function pop(component) { - const context_stack_item = component_context; - if (context_stack_item !== null) { - if (component !== undefined) { - context_stack_item.x = component; - } - const component_effects = context_stack_item.e; - if (component_effects !== null) { - var previous_effect = active_effect; - var previous_reaction = active_reaction; - context_stack_item.e = null; - try { - for (var i = 0; i < component_effects.length; i++) { - var component_effect = component_effects[i]; - set_active_effect(component_effect.effect); - set_active_reaction(component_effect.reaction); - effect(component_effect.fn); - } - } finally { - set_active_effect(previous_effect); - set_active_reaction(previous_reaction); - } - } - component_context = context_stack_item.p; - if (DEV) { - dev_current_component_function = context_stack_item.p?.function ?? null; - } - context_stack_item.m = true; - } - // Micro-optimization: Don't set .a above to the empty object - // so it can be garbage-collected when the return here is unused - return component || /** @type {T} */ ({}); -} - /** * Possibly traverse an object and read all its properties so that they're all reactive in case this is `$state`. * Does only check first level of an object for performance reasons (heuristic should be good for 99% of all cases). diff --git a/packages/svelte/src/internal/client/validate.js b/packages/svelte/src/internal/client/validate.js index 24e280edf8..ec3d805447 100644 --- a/packages/svelte/src/internal/client/validate.js +++ b/packages/svelte/src/internal/client/validate.js @@ -1,4 +1,4 @@ -import { dev_current_component_function } from './runtime.js'; +import { dev_current_component_function } from './context.js'; import { is_array } from '../shared/utils.js'; import * as e from './errors.js'; import { FILENAME } from '../../constants.js'; diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js index 3715617f4c..3a05bc0496 100644 --- a/packages/svelte/src/legacy/legacy-client.js +++ b/packages/svelte/src/legacy/legacy-client.js @@ -3,19 +3,13 @@ import { DIRTY, LEGACY_PROPS, MAYBE_DIRTY } from '../internal/client/constants.j import { user_pre_effect } from '../internal/client/reactivity/effects.js'; import { mutable_source, set } from '../internal/client/reactivity/sources.js'; import { hydrate, mount, unmount } from '../internal/client/render.js'; -import { - active_effect, - component_context, - dev_current_component_function, - flush_sync, - get, - set_signal_status -} from '../internal/client/runtime.js'; +import { active_effect, flush_sync, get, set_signal_status } from '../internal/client/runtime.js'; import { lifecycle_outside_component } from '../internal/shared/errors.js'; import { define_property, is_array } from '../internal/shared/utils.js'; import * as w from '../internal/client/warnings.js'; import { DEV } from 'esm-env'; import { FILENAME } from '../constants.js'; +import { component_context, dev_current_component_function } from '../internal/client/context.js'; /** * Takes the same options as a Svelte 4 component and the component function and returns a Svelte 4 compatible component. diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 5f0b93e136..aa1dbf3132 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1,6 +1,7 @@ import { describe, assert, it } from 'vitest'; import { flushSync } from '../../src/index-client'; import * as $ from '../../src/internal/client/runtime'; +import { push, pop } from '../../src/internal/client/context'; import { effect, effect_root, @@ -22,13 +23,13 @@ import { SvelteSet } from '../../src/reactivity/set'; function run_test(runes: boolean, fn: (runes: boolean) => () => void) { return () => { // Create a component context to test runes vs legacy mode - $.push({}, runes); + push({}, runes); // Create a render context so that effect validations etc don't fail let execute: any; const destroy = effect_root(() => { execute = fn(runes); }); - $.pop(); + pop(); execute(); destroy(); }; diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index d00b2b01ed..eb3e93e4b5 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -421,6 +421,34 @@ declare module 'svelte' { }): Snippet; /** Anything except a function */ type NotFunction = T extends Function ? never : T; + /** + * Retrieves the context that belongs to the closest parent component with the specified `key`. + * Must be called during component initialisation. + * + * */ + export function getContext(key: any): T; + /** + * Associates an arbitrary `context` object with the current component and the specified `key` + * and returns that object. The context is then available to children of the component + * (including slotted content) with `getContext`. + * + * Like lifecycle functions, this must be called during component initialisation. + * + * */ + export function setContext(key: any, context: T): T; + /** + * Checks whether a given `key` has been set in the context of a parent component. + * Must be called during component initialisation. + * + * */ + export function hasContext(key: any): boolean; + /** + * Retrieves the whole context map that belongs to the closest parent component. + * Must be called during component initialisation. Useful, for example, if you + * programmatically create a component and want to pass the existing context to it. + * + * */ + export function getAllContexts = Map>(): T; /** * Mounts a component to the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component. * Transitions will play during the initial render unless the `intro` option is set to `false`. @@ -484,34 +512,6 @@ declare module 'svelte' { * ``` * */ export function untrack(fn: () => T): T; - /** - * Retrieves the context that belongs to the closest parent component with the specified `key`. - * Must be called during component initialisation. - * - * */ - export function getContext(key: any): T; - /** - * Associates an arbitrary `context` object with the current component and the specified `key` - * and returns that object. The context is then available to children of the component - * (including slotted content) with `getContext`. - * - * Like lifecycle functions, this must be called during component initialisation. - * - * */ - export function setContext(key: any, context: T): T; - /** - * Checks whether a given `key` has been set in the context of a parent component. - * Must be called during component initialisation. - * - * */ - export function hasContext(key: any): boolean; - /** - * Retrieves the whole context map that belongs to the closest parent component. - * Must be called during component initialisation. Useful, for example, if you - * programmatically create a component and want to pass the existing context to it. - * - * */ - export function getAllContexts = Map>(): T; type Getters = { [K in keyof T]: () => T[K]; }; From 9410ad0318587c338c539ca7cbca1ff9d61db6a0 Mon Sep 17 00:00:00 2001 From: 7nik Date: Wed, 29 Jan 2025 13:50:07 +0200 Subject: [PATCH 2/5] fix: correctly look for sibling elements inside blocks and components (#15106) Fixes #15027 and fixes #14995 When the target element is inside a block or component, get_following_sibling_elements was looking only for sibling elements after the block/component. This PR fixes it by starting the search from the begging of the block/component and skipping nodes that go before the target element. --- .changeset/curly-balloons-relate.md | 5 + .../phases/2-analyze/css/css-prune.js | 25 ++-- .../svelte/tests/css/samples/has/_config.js | 118 ++++++++++-------- .../svelte/tests/css/samples/has/expected.css | 10 ++ .../svelte/tests/css/samples/has/input.svelte | 12 ++ 5 files changed, 109 insertions(+), 61 deletions(-) create mode 100644 .changeset/curly-balloons-relate.md diff --git a/.changeset/curly-balloons-relate.md b/.changeset/curly-balloons-relate.md new file mode 100644 index 0000000000..69d13b8692 --- /dev/null +++ b/.changeset/curly-balloons-relate.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly look for sibling elements inside blocks and components when pruning CSS diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index ca7476ef7f..109010e88c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -638,19 +638,30 @@ function get_following_sibling_elements(element, include_self) { /** @type {Array} */ const siblings = []; - // ...then walk them, starting from the node after the one - // containing the element in question + // ...then walk them, starting from the node containing the element in question + // skipping nodes that appears before the element const seen = new Set(); + let skip = true; /** @param {Compiler.AST.SvelteNode} node */ function get_siblings(node) { walk(node, null, { RegularElement(node) { - siblings.push(node); + if (node === element) { + skip = false; + if (include_self) siblings.push(node); + } else if (!skip) { + siblings.push(node); + } }, SvelteElement(node) { - siblings.push(node); + if (node === element) { + skip = false; + if (include_self) siblings.push(node); + } else if (!skip) { + siblings.push(node); + } }, RenderTag(node) { for (const snippet of node.metadata.snippets) { @@ -663,14 +674,10 @@ function get_following_sibling_elements(element, include_self) { }); } - for (const node of nodes.slice(nodes.indexOf(start) + 1)) { + for (const node of nodes.slice(nodes.indexOf(start))) { get_siblings(node); } - if (include_self) { - siblings.push(element); - } - return siblings; } diff --git a/packages/svelte/tests/css/samples/has/_config.js b/packages/svelte/tests/css/samples/has/_config.js index e5dc5f3459..33bbe74949 100644 --- a/packages/svelte/tests/css/samples/has/_config.js +++ b/packages/svelte/tests/css/samples/has/_config.js @@ -6,182 +6,196 @@ export default test({ code: 'css_unused_selector', message: 'Unused CSS selector ".unused:has(y)"', start: { - line: 31, + line: 33, column: 1, - character: 308 + character: 330 }, end: { - line: 31, + line: 33, column: 15, - character: 322 + character: 344 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused:has(:global(y))"', start: { - line: 34, + line: 36, column: 1, - character: 343 + character: 365 }, end: { - line: 34, + line: 36, column: 24, - character: 366 + character: 388 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(.unused)"', start: { - line: 37, + line: 39, column: 1, - character: 387 + character: 409 }, end: { - line: 37, + line: 39, column: 15, - character: 401 + character: 423 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ":global(.foo):has(.unused)"', start: { - line: 40, + line: 42, column: 1, - character: 422 + character: 444 }, end: { - line: 40, + line: 42, column: 27, - character: 448 + character: 470 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(y):has(.unused)"', start: { - line: 50, + line: 52, column: 1, - character: 556 + character: 578 }, end: { - line: 50, + line: 52, column: 22, - character: 577 + character: 599 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused"', start: { - line: 69, + line: 71, column: 2, - character: 782 + character: 804 }, end: { - line: 69, + line: 71, column: 9, - character: 789 + character: 811 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused x:has(y)"', start: { - line: 85, + line: 87, column: 1, - character: 936 + character: 958 }, end: { - line: 85, + line: 87, column: 17, - character: 952 + character: 974 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ".unused:has(.unused)"', start: { - line: 88, + line: 90, column: 1, - character: 973 + character: 995 }, end: { - line: 88, + line: 90, column: 21, - character: 993 + character: 1015 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(> z)"', start: { - line: 98, + line: 100, column: 1, - character: 1093 + character: 1115 }, end: { - line: 98, + line: 100, column: 11, - character: 1103 + character: 1125 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(> d)"', start: { - line: 101, + line: 103, column: 1, - character: 1124 + character: 1146 }, end: { - line: 101, + line: 103, column: 11, - character: 1134 + character: 1156 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "x:has(~ y)"', start: { - line: 121, + line: 123, column: 1, - character: 1326 + character: 1348 }, end: { - line: 121, + line: 123, column: 11, - character: 1336 + character: 1358 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector "f:has(~ d)"', + start: { + line: 133, + column: 1, + character: 1446 + }, + end: { + line: 133, + column: 11, + character: 1456 } }, { code: 'css_unused_selector', message: 'Unused CSS selector ":has(.unused)"', start: { - line: 129, + line: 141, column: 2, - character: 1409 + character: 1529 }, end: { - line: 129, + line: 141, column: 15, - character: 1422 + character: 1542 } }, { code: 'css_unused_selector', message: 'Unused CSS selector "&:has(.unused)"', start: { - line: 135, + line: 147, column: 2, - character: 1480 + character: 1600 }, end: { - line: 135, + line: 147, column: 16, - character: 1494 + character: 1614 } } ] diff --git a/packages/svelte/tests/css/samples/has/expected.css b/packages/svelte/tests/css/samples/has/expected.css index 68d6aad68a..9627bf730c 100644 --- a/packages/svelte/tests/css/samples/has/expected.css +++ b/packages/svelte/tests/css/samples/has/expected.css @@ -112,6 +112,16 @@ color: red; }*/ + d.svelte-xyz:has(+ e:where(.svelte-xyz)) { + color: green; + } + d.svelte-xyz:has(~ f:where(.svelte-xyz)) { + color: green; + } + /* (unused) f:has(~ d) { + color: red; + }*/ + .foo { .svelte-xyz:has(x:where(.svelte-xyz)) { color: green; diff --git a/packages/svelte/tests/css/samples/has/input.svelte b/packages/svelte/tests/css/samples/has/input.svelte index 3487b64e8c..946cf2df90 100644 --- a/packages/svelte/tests/css/samples/has/input.svelte +++ b/packages/svelte/tests/css/samples/has/input.svelte @@ -3,6 +3,8 @@ {#if foo} + + {/if} @@ -122,6 +124,16 @@ color: red; } + d:has(+ e) { + color: green; + } + d:has(~ f) { + color: green; + } + f:has(~ d) { + color: red; + } + :global(.foo) { :has(x) { color: green; From 13a6d555c05f0833d828a518f70722dba351fabc Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 29 Jan 2025 13:17:12 +0000 Subject: [PATCH 3/5] fix: improve derived connection to ownership graph (#15137) * fix: improve derived connection to ownership graph * revised * revised * revised * revised * feedback * feedback * invasive change * fix bugs * fix other bug --- .changeset/blue-rocks-play.md | 5 + .../internal/client/reactivity/deriveds.js | 42 ++-- .../src/internal/client/reactivity/effects.js | 22 +- .../src/internal/client/reactivity/types.d.ts | 6 +- .../svelte/src/internal/client/runtime.js | 46 ++-- packages/svelte/tests/signals/test.ts | 201 +++++++++++++----- 6 files changed, 199 insertions(+), 123 deletions(-) create mode 100644 .changeset/blue-rocks-play.md diff --git a/.changeset/blue-rocks-play.md b/.changeset/blue-rocks-play.md new file mode 100644 index 0000000000..31e9991148 --- /dev/null +++ b/.changeset/blue-rocks-play.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: improve derived connection to ownership graph diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index e86963408c..1f65ff38c3 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -35,8 +35,12 @@ import { component_context } from '../context.js'; /*#__NO_SIDE_EFFECTS__*/ export function derived(fn) { var flags = DERIVED | DIRTY; + var parent_derived = + active_reaction !== null && (active_reaction.f & DERIVED) !== 0 + ? /** @type {Derived} */ (active_reaction) + : null; - if (active_effect === null) { + if (active_effect === null || (parent_derived !== null && (parent_derived.f & UNOWNED) !== 0)) { flags |= UNOWNED; } else { // Since deriveds are evaluated lazily, any effects created inside them are @@ -44,16 +48,11 @@ export function derived(fn) { active_effect.f |= EFFECT_HAS_DERIVED; } - var parent_derived = - active_reaction !== null && (active_reaction.f & DERIVED) !== 0 - ? /** @type {Derived} */ (active_reaction) - : null; - /** @type {Derived} */ const signal = { - children: null, ctx: component_context, deps: null, + effects: null, equals, f: flags, fn, @@ -87,19 +86,14 @@ export function derived_safe_equal(fn) { * @param {Derived} derived * @returns {void} */ -function destroy_derived_children(derived) { - var children = derived.children; - - if (children !== null) { - derived.children = null; - - for (var i = 0; i < children.length; i += 1) { - var child = children[i]; - if ((child.f & DERIVED) !== 0) { - destroy_derived(/** @type {Derived} */ (child)); - } else { - destroy_effect(/** @type {Effect} */ (child)); - } +export function destroy_derived_effects(derived) { + var effects = derived.effects; + + if (effects !== null) { + derived.effects = null; + + for (var i = 0; i < effects.length; i += 1) { + destroy_effect(/** @type {Effect} */ (effects[i])); } } } @@ -147,7 +141,7 @@ export function execute_derived(derived) { stack.push(derived); - destroy_derived_children(derived); + destroy_derived_effects(derived); value = update_reaction(derived); } finally { set_active_effect(prev_active_effect); @@ -156,7 +150,7 @@ export function execute_derived(derived) { } } else { try { - destroy_derived_children(derived); + destroy_derived_effects(derived); value = update_reaction(derived); } finally { set_active_effect(prev_active_effect); @@ -188,9 +182,9 @@ export function update_derived(derived) { * @returns {void} */ export function destroy_derived(derived) { - destroy_derived_children(derived); + destroy_derived_effects(derived); remove_reactions(derived, 0); set_signal_status(derived, DESTROYED); - derived.v = derived.children = derived.deps = derived.ctx = derived.reactions = null; + derived.v = derived.deps = derived.ctx = derived.reactions = null; } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 8a7cffecd6..d014ff793d 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -53,7 +53,7 @@ export function validate_effect(rune) { e.effect_orphan(rune); } - if (active_reaction !== null && (active_reaction.f & UNOWNED) !== 0) { + if (active_reaction !== null && (active_reaction.f & UNOWNED) !== 0 && active_effect === null) { e.effect_in_unowned_derived(); } @@ -99,7 +99,6 @@ function create_effect(type, fn, sync, push = true) { var effect = { ctx: component_context, deps: null, - deriveds: null, nodes_start: null, nodes_end: null, f: type | DIRTY, @@ -153,7 +152,7 @@ function create_effect(type, fn, sync, push = true) { // if we're in a derived, add the effect there too if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) { var derived = /** @type {Derived} */ (active_reaction); - (derived.children ??= []).push(effect); + (derived.effects ??= []).push(effect); } } @@ -395,22 +394,6 @@ export function execute_effect_teardown(effect) { } } -/** - * @param {Effect} signal - * @returns {void} - */ -export function destroy_effect_deriveds(signal) { - var deriveds = signal.deriveds; - - if (deriveds !== null) { - signal.deriveds = null; - - for (var i = 0; i < deriveds.length; i += 1) { - destroy_derived(deriveds[i]); - } - } -} - /** * @param {Effect} signal * @param {boolean} remove_dom @@ -468,7 +451,6 @@ export function destroy_effect(effect, remove_dom = true) { } destroy_effect_children(effect, remove_dom && !removed); - destroy_effect_deriveds(effect); remove_reactions(effect, 0); set_signal_status(effect, DESTROYED); diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 3a76a3ff83..5ef0097649 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -36,8 +36,8 @@ export interface Reaction extends Signal { export interface Derived extends Value, Reaction { /** The derived function */ fn: () => V; - /** Reactions created inside this signal */ - children: null | Reaction[]; + /** Effects created inside this signal */ + effects: null | Effect[]; /** Parent effect or derived */ parent: Effect | Derived | null; } @@ -51,8 +51,6 @@ export interface Effect extends Reaction { */ nodes_start: null | TemplateNode; nodes_end: null | TemplateNode; - /** Reactions created inside this signal */ - deriveds: null | Derived[]; /** The effect function */ fn: null | (() => void | (() => void)); /** The teardown function returned from the effect function */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 859925186f..2c53ac8f7b 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -4,7 +4,6 @@ import { define_property, get_descriptors, get_prototype_of, index_of } from '.. import { destroy_block_effect_children, destroy_effect_children, - destroy_effect_deriveds, execute_effect_teardown, unlink_effect } from './reactivity/effects.js'; @@ -28,7 +27,12 @@ import { } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { internal_set, set } from './reactivity/sources.js'; -import { destroy_derived, execute_derived, update_derived } from './reactivity/deriveds.js'; +import { + destroy_derived, + destroy_derived_effects, + execute_derived, + update_derived +} from './reactivity/deriveds.js'; import * as e from './errors.js'; import { FILENAME } from '../../constants.js'; import { legacy_mode_flag, tracing_mode_flag } from '../flags/index.js'; @@ -409,7 +413,16 @@ export function update_reaction(reaction) { skipped_deps = 0; untracked_writes = null; active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null; - skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0; + // prettier-ignore + skip_reaction = + (flags & UNOWNED) !== 0 && + (!is_flushing_effect || + // If we were previously not in a reactive context and we're reading an unowned derived + // that was created inside another reaction, then we don't fully know the real owner and thus + // we need to skip adding any reactions for this unowned + ((previous_reaction === null || previous_untracking) && + /** @type {Derived} */ (reaction).parent !== null)); + derived_sources = null; set_component_context(reaction.ctx); untracking = false; @@ -517,6 +530,8 @@ function remove_reaction(signal, dependency) { if ((dependency.f & (UNOWNED | DISCONNECTED)) === 0) { dependency.f ^= DISCONNECTED; } + // Disconnect any reactions owned by this reaction + destroy_derived_effects(/** @type {Derived} **/ (dependency)); remove_reactions(/** @type {Derived} **/ (dependency), 0); } } @@ -564,7 +579,6 @@ export function update_effect(effect) { } else { destroy_effect_children(effect); } - destroy_effect_deriveds(effect); execute_effect_teardown(effect); var teardown = update_reaction(effect); @@ -934,30 +948,20 @@ export function get(signal) { new_deps.push(signal); } } - } - - if ( + } else if ( is_derived && /** @type {Derived} */ (signal).deps === null && - (active_reaction === null || untracking || (active_reaction.f & DERIVED) !== 0) + /** @type {Derived} */ (signal).effects === null ) { var derived = /** @type {Derived} */ (signal); var parent = derived.parent; if (parent !== null) { - // Attach the derived to the nearest parent effect or derived - if ((parent.f & DERIVED) !== 0) { - var parent_derived = /** @type {Derived} */ (parent); - - if (!parent_derived.children?.includes(derived)) { - (parent_derived.children ??= []).push(derived); - } - } else { - var parent_effect = /** @type {Effect} */ (parent); - - if (!parent_effect.deriveds?.includes(derived)) { - (parent_effect.deriveds ??= []).push(derived); - } + // If the derived is owned by another derived then mark it as unowned + // as the derived value might have been referenced in a different context + // since and thus its parent might not be its true owner anymore + if ((parent.f & UNOWNED) === 0) { + derived.f ^= UNOWNED; } } } diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index aa1dbf3132..2ce624a777 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -9,11 +9,12 @@ import { user_effect } from '../../src/internal/client/reactivity/effects'; import { state, set } from '../../src/internal/client/reactivity/sources'; -import type { Derived, Value } from '../../src/internal/client/types'; +import type { Derived, Effect, Value } from '../../src/internal/client/types'; import { proxy } from '../../src/internal/client/proxy'; import { derived } from '../../src/internal/client/reactivity/deriveds'; import { snapshot } from '../../src/internal/shared/clone.js'; import { SvelteSet } from '../../src/reactivity/set'; +import { DESTROYED } from '../../src/internal/client/constants'; /** * @param runes runes mode @@ -68,7 +69,7 @@ describe('signals', () => { }; }); - test('multiple effects with state and derived in it#1', () => { + test('multiple effects with state and derived in it #1', () => { const log: string[] = []; let count = state(0); @@ -89,7 +90,7 @@ describe('signals', () => { }; }); - test('multiple effects with state and derived in it#2', () => { + test('multiple effects with state and derived in it #2', () => { const log: string[] = []; let count = state(0); @@ -256,12 +257,16 @@ describe('signals', () => { const a = state(0); const b = state(0); - const c = derived(() => { - const a_2 = derived(() => $.get(a) + '!'); - const b_2 = derived(() => $.get(b) + '?'); - nested.push(a_2, b_2); + let c: any; + + const destroy = effect_root(() => { + c = derived(() => { + const a_2 = derived(() => $.get(a) + '!'); + const b_2 = derived(() => $.get(b) + '?'); + nested.push(a_2, b_2); - return { a: $.get(a_2), b: $.get(b_2) }; + return { a: $.get(a_2), b: $.get(b_2) }; + }); }); $.get(c); @@ -274,11 +279,10 @@ describe('signals', () => { $.get(c); - // Ensure we're not leaking dependencies - assert.deepEqual( - nested.slice(0, -2).map((s) => s.deps), - [null, null, null, null] - ); + destroy(); + + assert.equal(a.reactions, null); + assert.equal(b.reactions, null); }; }); @@ -477,6 +481,7 @@ describe('signals', () => { effect(() => { log.push('inner', $.get(inner)); }); + return $.get(outer); }); }); }); @@ -530,6 +535,103 @@ describe('signals', () => { }; }); + test('mixed nested deriveds correctly cleanup when no longer connected to graph #1', () => { + let a: Derived; + let b: Derived; + let s = state(0); + + const destroy = effect_root(() => { + render_effect(() => { + a = derived(() => { + b = derived(() => { + $.get(s); + }); + $.untrack(() => { + $.get(b); + }); + $.get(s); + }); + $.get(a); + }); + }); + + return () => { + flushSync(); + assert.equal(a?.deps?.length, 1); + assert.equal(s?.reactions?.length, 1); + destroy(); + assert.equal(s?.reactions, null); + }; + }); + + test('mixed nested deriveds correctly cleanup when no longer connected to graph #2', () => { + let a: Derived; + let b: Derived; + let s = state(0); + + const destroy = effect_root(() => { + render_effect(() => { + a = derived(() => { + b = derived(() => { + $.get(s); + }); + effect_root(() => { + $.get(b); + }); + $.get(s); + }); + $.get(a); + }); + }); + + return () => { + flushSync(); + assert.equal(a?.deps?.length, 1); + assert.equal(s?.reactions?.length, 1); + destroy(); + assert.equal(s?.reactions, null); + }; + }); + + test('mixed nested deriveds correctly cleanup when no longer connected to graph #3', () => { + let a: Derived; + let b: Derived; + let s = state(0); + let logs: any[] = []; + + const destroy = effect_root(() => { + render_effect(() => { + a = derived(() => { + b = derived(() => { + return $.get(s); + }); + effect_root(() => { + $.get(b); + }); + render_effect(() => { + logs.push($.get(b)); + }); + $.get(s); + }); + $.get(a); + }); + }); + + return () => { + flushSync(); + assert.equal(a?.deps?.length, 1); + assert.equal(s?.reactions?.length, 2); + + set(s, 1); + flushSync(); + + assert.deepEqual(logs, [0, 1]); + + destroy(); + assert.equal(s?.reactions, null); + }; + }); + test('deriveds update upon reconnection #1', () => { let a = state(false); let b = state(false); @@ -778,56 +880,44 @@ describe('signals', () => { }; }); - test('nested deriveds clean up the relationships when used with untrack', () => { + test('deriveds containing effects work correctly', () => { return () => { let a = render_effect(() => {}); + let b = state(0); + let c; + let effects: Effect[] = []; const destroy = effect_root(() => { a = render_effect(() => { - $.untrack(() => { - const b = derived(() => { - const c = derived(() => {}); - $.untrack(() => { - $.get(c); - }); - }); + c = derived(() => { + effects.push( + effect(() => { + $.get(b); + }) + ); $.get(b); }); + $.get(c); }); }); - assert.deepEqual(a.deriveds?.length, 1); - - destroy(); + assert.equal(c!.effects?.length, 1); + assert.equal(a.first, a.last); - assert.deepEqual(a.deriveds, null); - }; - }); - - test('nested deriveds do not connect inside parent deriveds if unused', () => { - return () => { - let a = render_effect(() => {}); - let b: Derived | undefined; + set(b, 1); - const destroy = effect_root(() => { - a = render_effect(() => { - $.untrack(() => { - b = derived(() => { - derived(() => {}); - derived(() => {}); - derived(() => {}); - }); - $.get(b); - }); - }); - }); + flushSync(); - assert.deepEqual(a.deriveds?.length, 1); - assert.deepEqual(b?.children, null); + assert.equal(c!.effects?.length, 1); + assert.equal(a.first, a.last); destroy(); - assert.deepEqual(a.deriveds, null); + assert.equal(a.first, null); + + assert.equal(effects.length, 2); + assert.equal((effects[0].f & DESTROYED) !== 0, true); + assert.equal((effects[1].f & DESTROYED) !== 0, true); }; }); @@ -836,7 +926,7 @@ describe('signals', () => { let a = render_effect(() => {}); let b = state(0); let c; - let effects = []; + let effects: Effect[] = []; const destroy = effect_root(() => { a = render_effect(() => { @@ -854,20 +944,23 @@ describe('signals', () => { }); }); - assert.deepEqual(c!.children?.length, 1); - assert.deepEqual(a.first, a.last); + assert.equal(c!.effects?.length, 1); + assert.equal(a.first, a.last); set(b, 1); flushSync(); - assert.deepEqual(c!.children?.length, 1); - assert.deepEqual(a.first, a.last); + assert.equal(c!.effects?.length, 1); + assert.equal(a.first, a.last); destroy(); - assert.deepEqual(a.deriveds, null); - assert.deepEqual(a.first, null); + assert.equal(a.first, null); + + assert.equal(effects.length, 2); + assert.equal((effects[0].f & DESTROYED) !== 0, true); + assert.equal((effects[1].f & DESTROYED) !== 0, true); }; }); From c8bbb15693c59a3a5b3d7478d7adc1b0fa089ba7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 29 Jan 2025 13:18:43 +0000 Subject: [PATCH 4/5] Version Packages (#15139) Co-authored-by: github-actions[bot] --- .changeset/blue-rocks-play.md | 5 ----- .changeset/curly-balloons-relate.md | 5 ----- packages/svelte/CHANGELOG.md | 8 ++++++++ packages/svelte/package.json | 2 +- packages/svelte/src/version.js | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) delete mode 100644 .changeset/blue-rocks-play.md delete mode 100644 .changeset/curly-balloons-relate.md diff --git a/.changeset/blue-rocks-play.md b/.changeset/blue-rocks-play.md deleted file mode 100644 index 31e9991148..0000000000 --- a/.changeset/blue-rocks-play.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: improve derived connection to ownership graph diff --git a/.changeset/curly-balloons-relate.md b/.changeset/curly-balloons-relate.md deleted file mode 100644 index 69d13b8692..0000000000 --- a/.changeset/curly-balloons-relate.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: correctly look for sibling elements inside blocks and components when pruning CSS diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index 3b42ad6927..bb48391e0e 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,13 @@ # svelte +## 5.19.5 + +### Patch Changes + +- fix: improve derived connection to ownership graph ([#15137](https://github.com/sveltejs/svelte/pull/15137)) + +- fix: correctly look for sibling elements inside blocks and components when pruning CSS ([#15106](https://github.com/sveltejs/svelte/pull/15106)) + ## 5.19.4 ### Patch Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index f94992e42e..a44ba43364 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.19.4", + "version": "5.19.5", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/version.js b/packages/svelte/src/version.js index b45197aa9c..a509ef3fec 100644 --- a/packages/svelte/src/version.js +++ b/packages/svelte/src/version.js @@ -4,5 +4,5 @@ * The current version, as set in package.json. * @type {string} */ -export const VERSION = '5.19.4'; +export const VERSION = '5.19.5'; export const PUBLIC_VERSION = '5'; From 8e83127e1a84f173855dd14d46efb402ce3523dc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 29 Jan 2025 09:22:35 -0500 Subject: [PATCH 5/5] chore: move more code (#15133) * move more code * lint --- packages/svelte/src/index-client.js | 35 +++++++++ .../svelte/src/internal/client/context.js | 5 ++ .../src/internal/client/dom/blocks/await.js | 3 +- .../src/internal/client/dom/blocks/key.js | 2 +- .../client/dom/elements/bindings/input.js | 3 +- packages/svelte/src/internal/client/index.js | 4 +- .../src/internal/client/reactivity/props.js | 11 +-- .../src/internal/client/reactivity/sources.js | 32 +++++++- .../svelte/src/internal/client/runtime.js | 73 +------------------ packages/svelte/tests/signals/test.ts | 10 +-- 10 files changed, 86 insertions(+), 92 deletions(-) diff --git a/packages/svelte/src/index-client.js b/packages/svelte/src/index-client.js index bd2b353395..ca29d5bfbe 100644 --- a/packages/svelte/src/index-client.js +++ b/packages/svelte/src/index-client.js @@ -8,6 +8,41 @@ import * as e from './internal/client/errors.js'; import { lifecycle_outside_component } from './internal/shared/errors.js'; import { legacy_mode_flag } from './internal/flags/index.js'; import { component_context } from './internal/client/context.js'; +import { DEV } from 'esm-env'; + +if (DEV) { + /** + * @param {string} rune + */ + function throw_rune_error(rune) { + if (!(rune in globalThis)) { + // TODO if people start adjusting the "this can contain runes" config through v-p-s more, adjust this message + /** @type {any} */ + let value; // let's hope noone modifies this global, but belts and braces + Object.defineProperty(globalThis, rune, { + configurable: true, + // eslint-disable-next-line getter-return + get: () => { + if (value !== undefined) { + return value; + } + + e.rune_outside_svelte(rune); + }, + set: (v) => { + value = v; + } + }); + } + } + + throw_rune_error('$state'); + throw_rune_error('$effect'); + throw_rune_error('$derived'); + throw_rune_error('$inspect'); + throw_rune_error('$props'); + throw_rune_error('$bindable'); +} /** * The `onMount` function schedules a callback to run as soon as the component has been mounted to the DOM. diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js index 1d7f0747cd..e1088edf30 100644 --- a/packages/svelte/src/internal/client/context.js +++ b/packages/svelte/src/internal/client/context.js @@ -185,6 +185,11 @@ export function pop(component) { return component || /** @type {T} */ ({}); } +/** @returns {boolean} */ +export function is_runes() { + return !legacy_mode_flag || (component_context !== null && component_context.l === null); +} + /** * @param {string} name * @returns {Map} diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 633a6e5bab..c8c7c1c0ea 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -3,12 +3,13 @@ import { DEV } from 'esm-env'; import { is_promise } from '../../../shared/utils.js'; import { block, branch, pause_effect, resume_effect } from '../../reactivity/effects.js'; import { internal_set, mutable_source, source } from '../../reactivity/sources.js'; -import { flush_sync, is_runes, set_active_effect, set_active_reaction } from '../../runtime.js'; +import { flush_sync, set_active_effect, set_active_reaction } from '../../runtime.js'; import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; import { queue_micro_task } from '../task.js'; import { UNINITIALIZED } from '../../../../constants.js'; import { component_context, + is_runes, set_component_context, set_dev_current_component_function } from '../../context.js'; diff --git a/packages/svelte/src/internal/client/dom/blocks/key.js b/packages/svelte/src/internal/client/dom/blocks/key.js index 4a8b7b94fc..a697163548 100644 --- a/packages/svelte/src/internal/client/dom/blocks/key.js +++ b/packages/svelte/src/internal/client/dom/blocks/key.js @@ -2,7 +2,7 @@ import { UNINITIALIZED } from '../../../../constants.js'; import { block, branch, pause_effect } from '../../reactivity/effects.js'; import { not_equal, safe_not_equal } from '../../reactivity/equality.js'; -import { is_runes } from '../../runtime.js'; +import { is_runes } from '../../context.js'; import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; /** 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 3ea1a24d7e..f1992007ed 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -5,7 +5,8 @@ import * as e from '../../../errors.js'; import { is } from '../../../proxy.js'; import { queue_micro_task } from '../../task.js'; import { hydrating } from '../../hydration.js'; -import { is_runes, untrack } from '../../../runtime.js'; +import { untrack } from '../../../runtime.js'; +import { is_runes } from '../../../context.js'; /** * @param {HTMLInputElement} input diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 877f3c59ee..3f3b29b029 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -110,7 +110,7 @@ export { user_effect, user_pre_effect } from './reactivity/effects.js'; -export { mutable_state, mutate, set, state } from './reactivity/sources.js'; +export { mutable_state, mutate, set, state, update, update_pre } from './reactivity/sources.js'; export { prop, rest_props, @@ -139,8 +139,6 @@ export { flush_sync, tick, untrack, - update, - update_pre, exclude_from_object, deep_read, deep_read_state diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 3e5a0258c7..d157642d5d 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -8,16 +8,9 @@ import { PROPS_IS_UPDATED } from '../../../constants.js'; import { get_descriptor, is_function } from '../../shared/utils.js'; -import { mutable_source, set, source } from './sources.js'; +import { mutable_source, set, source, update } from './sources.js'; import { derived, derived_safe_equal } from './deriveds.js'; -import { - active_effect, - get, - captured_signals, - set_active_effect, - untrack, - update -} from '../runtime.js'; +import { active_effect, get, captured_signals, set_active_effect, untrack } from '../runtime.js'; import { safe_equals } from './equality.js'; import * as e from '../errors.js'; import { diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index b3e66a3336..ded0ca0584 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -5,7 +5,6 @@ import { active_effect, untracked_writes, get, - is_runes, schedule_effect, set_untracked_writes, set_signal_status, @@ -34,7 +33,7 @@ import { import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { get_stack } from '../dev/tracing.js'; -import { component_context } from '../context.js'; +import { component_context, is_runes } from '../context.js'; export let inspect_effects = new Set(); @@ -226,6 +225,35 @@ export function internal_set(source, value) { return value; } +/** + * @template {number | bigint} T + * @param {Source} source + * @param {1 | -1} [d] + * @returns {T} + */ +export function update(source, d = 1) { + var value = get(source); + var result = d === 1 ? value++ : value--; + + set(source, value); + + // @ts-expect-error + return result; +} + +/** + * @template {number | bigint} T + * @param {Source} source + * @param {1 | -1} [d] + * @returns {T} + */ +export function update_pre(source, d = 1) { + var value = get(source); + + // @ts-expect-error + return set(source, d === 1 ? ++value : --value); +} + /** * @param {Value} signal * @param {number} status should be DIRTY or MAYBE_DIRTY diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2c53ac8f7b..bf9d17fa23 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -26,7 +26,7 @@ import { BOUNDARY_EFFECT } from './constants.js'; import { flush_tasks } from './dom/task.js'; -import { internal_set, set } from './reactivity/sources.js'; +import { internal_set } from './reactivity/sources.js'; import { destroy_derived, destroy_derived_effects, @@ -35,11 +35,12 @@ import { } from './reactivity/deriveds.js'; import * as e from './errors.js'; import { FILENAME } from '../../constants.js'; -import { legacy_mode_flag, tracing_mode_flag } from '../flags/index.js'; +import { tracing_mode_flag } from '../flags/index.js'; import { tracing_expressions, get_stack } from './dev/tracing.js'; import { component_context, dev_current_component_function, + is_runes, set_component_context, set_dev_current_component_function } from './context.js'; @@ -161,11 +162,6 @@ export function increment_write_version() { return ++write_version; } -/** @returns {boolean} */ -export function is_runes() { - return !legacy_mode_flag || (component_context !== null && component_context.l === null); -} - /** * Determines whether a derived or effect is dirty. * If it is MAYBE_DIRTY, will set the status to CLEAN @@ -1095,35 +1091,6 @@ export function set_signal_status(signal, status) { signal.f = (signal.f & STATUS_MASK) | status; } -/** - * @template {number | bigint} T - * @param {Value} signal - * @param {1 | -1} [d] - * @returns {T} - */ -export function update(signal, d = 1) { - var value = get(signal); - var result = d === 1 ? value++ : value--; - - set(signal, value); - - // @ts-expect-error - return result; -} - -/** - * @template {number | bigint} T - * @param {Value} signal - * @param {1 | -1} [d] - * @returns {T} - */ -export function update_pre(signal, d = 1) { - var value = get(signal); - - // @ts-expect-error - return set(signal, d === 1 ? ++value : --value); -} - /** * @param {Record} obj * @param {string[]} keys @@ -1215,37 +1182,3 @@ export function deep_read(value, visited = new Set()) { } } } - -if (DEV) { - /** - * @param {string} rune - */ - function throw_rune_error(rune) { - if (!(rune in globalThis)) { - // TODO if people start adjusting the "this can contain runes" config through v-p-s more, adjust this message - /** @type {any} */ - let value; // let's hope noone modifies this global, but belts and braces - Object.defineProperty(globalThis, rune, { - configurable: true, - // eslint-disable-next-line getter-return - get: () => { - if (value !== undefined) { - return value; - } - - e.rune_outside_svelte(rune); - }, - set: (v) => { - value = v; - } - }); - } - } - - throw_rune_error('$state'); - throw_rune_error('$effect'); - throw_rune_error('$derived'); - throw_rune_error('$inspect'); - throw_rune_error('$props'); - throw_rune_error('$bindable'); -} diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 2ce624a777..bd9bc50ae3 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -8,7 +8,7 @@ import { render_effect, user_effect } from '../../src/internal/client/reactivity/effects'; -import { state, set } from '../../src/internal/client/reactivity/sources'; +import { state, set, update, update_pre } from '../../src/internal/client/reactivity/sources'; import type { Derived, Effect, Value } from '../../src/internal/client/types'; import { proxy } from '../../src/internal/client/proxy'; import { derived } from '../../src/internal/client/reactivity/deriveds'; @@ -968,14 +968,14 @@ describe('signals', () => { return () => { const count = state(0n); - assert.doesNotThrow(() => $.update(count)); + assert.doesNotThrow(() => update(count)); assert.equal($.get(count), 1n); - assert.doesNotThrow(() => $.update(count, -1)); + assert.doesNotThrow(() => update(count, -1)); assert.equal($.get(count), 0n); - assert.doesNotThrow(() => $.update_pre(count)); + assert.doesNotThrow(() => update_pre(count)); assert.equal($.get(count), 1n); - assert.doesNotThrow(() => $.update_pre(count, -1)); + assert.doesNotThrow(() => update_pre(count, -1)); assert.equal($.get(count), 0n); }; });