diff --git a/.changeset/lucky-drinks-push.md b/.changeset/lucky-drinks-push.md new file mode 100644 index 0000000000..f272bb0ac4 --- /dev/null +++ b/.changeset/lucky-drinks-push.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: handle `$$Props` interface during migration diff --git a/.changeset/plenty-rings-stare.md b/.changeset/plenty-rings-stare.md new file mode 100644 index 0000000000..2e9dfa5f48 --- /dev/null +++ b/.changeset/plenty-rings-stare.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: attach effects-inside-deriveds to the parent of the derived diff --git a/.changeset/silent-elephants-film.md b/.changeset/silent-elephants-film.md new file mode 100644 index 0000000000..1182b687f5 --- /dev/null +++ b/.changeset/silent-elephants-film.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: simplify and robustify appending styles diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index e4c759083d..c60d2c8719 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -335,7 +335,8 @@ const instance_script = { // } } - const binding = /** @type {Binding} */ (state.scope.get(declarator.id.name)); + const name = declarator.id.name; + const binding = /** @type {Binding} */ (state.scope.get(name)); if (state.analysis.uses_props && (declarator.init || binding.updated)) { throw new Error( @@ -343,19 +344,33 @@ const instance_script = { ); } - state.props.push({ - local: declarator.id.name, - exported: binding.prop_alias ? binding.prop_alias : declarator.id.name, - init: declarator.init + const prop = state.props.find((prop) => prop.exported === (binding.prop_alias || name)); + if (prop) { + // $$Props type was used + prop.init = declarator.init ? state.str.original.substring( /** @type {number} */ (declarator.init.start), /** @type {number} */ (declarator.init.end) ) - : '', - optional: !!declarator.init, - bindable: binding.updated, - ...extract_type_and_comment(declarator, state.str, path) - }); + : ''; + prop.bindable = binding.updated; + prop.exported = binding.prop_alias || name; + } else { + state.props.push({ + local: name, + exported: binding.prop_alias ? binding.prop_alias : name, + init: declarator.init + ? state.str.original.substring( + /** @type {number} */ (declarator.init.start), + /** @type {number} */ (declarator.init.end) + ) + : '', + optional: !!declarator.init, + bindable: binding.updated, + ...extract_type_and_comment(declarator, state.str, path) + }); + } + state.props_insertion_point = /** @type {number} */ (declarator.end); state.str.update( /** @type {number} */ (declarator.start), @@ -944,6 +959,48 @@ function handle_identifier(node, state, path) { } } // else passed as identifier, we don't know what to do here, so let it error + } else if ( + parent?.type === 'TSInterfaceDeclaration' || + parent?.type === 'TSTypeAliasDeclaration' + ) { + const members = + parent.type === 'TSInterfaceDeclaration' ? parent.body.body : parent.typeAnnotation?.members; + if (Array.isArray(members)) { + if (node.name === '$$Props') { + for (const member of members) { + const prop = state.props.find((prop) => prop.exported === member.key.name); + + const type = state.str.original.substring( + member.typeAnnotation.typeAnnotation.start, + member.typeAnnotation.typeAnnotation.end + ); + + let comment; + const comment_node = member.leadingComments?.at(-1); + if (comment_node?.type === 'Block') { + comment = state.str.original.substring(comment_node.start, comment_node.end); + } + + if (prop) { + prop.type = type; + prop.optional = member.optional; + prop.comment = comment ?? prop.comment; + } else { + state.props.push({ + local: member.key.name, + exported: member.key.name, + init: '', + bindable: false, + optional: member.optional, + type, + comment + }); + } + } + + state.str.remove(parent.start, parent.end); + } + } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 45e48e565e..04815dbc28 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -386,9 +386,7 @@ export function client_component(analysis, options) { state.hoisted.push(b.const('$$css', b.object([b.init('hash', hash), b.init('code', code)]))); component_block.body.unshift( - b.stmt( - b.call('$.append_styles', b.id('$$anchor'), b.id('$$css'), options.customElement && b.true) - ) + b.stmt(b.call('$.append_styles', b.id('$$anchor'), b.id('$$css'))) ); } diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index c1cf1e4055..95c8bd32e0 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -343,7 +343,14 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { // references Identifier(node, { path, state }) { const parent = path.at(-1); - if (parent && is_reference(node, /** @type {Node} */ (parent))) { + if ( + parent && + is_reference(node, /** @type {Node} */ (parent)) && + // TSTypeAnnotation, TSInterfaceDeclaration etc - these are normally already filtered out, + // but for the migration they aren't, so we need to filter them out here + // -> once migration script is gone we can remove this check + !parent.type.startsWith('TS') + ) { references.push([state.scope, { node, path: path.slice() }]); } }, diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 649a8b152f..15785d3848 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -19,6 +19,7 @@ export const LEGACY_DERIVED_PROP = 1 << 16; export const INSPECT_EFFECT = 1 << 17; export const HEAD_EFFECT = 1 << 18; export const EFFECT_HAS_DIRTY_CHILDREN = 1 << 19; +export const EFFECT_HAS_DERIVED = 1 << 20; export const STATE_SYMBOL = Symbol('$state'); export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index 2c7efe8994..52be36aa1f 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -2,25 +2,11 @@ import { DEV } from 'esm-env'; import { queue_micro_task } from './task.js'; import { register_style } from '../dev/css.js'; -var roots = new WeakMap(); - /** * @param {Node} anchor * @param {{ hash: string, code: string }} css - * @param {boolean} [is_custom_element] */ -export function append_styles(anchor, css, is_custom_element) { - // in dev, always check the DOM, so that styles can be replaced with HMR - if (!DEV && !is_custom_element) { - var doc = /** @type {Document} */ (anchor.ownerDocument); - - if (!roots.has(doc)) roots.set(doc, new Set()); - const seen = roots.get(doc); - - if (seen.has(css)) return; - seen.add(css); - } - +export function append_styles(anchor, css) { // Use `queue_micro_task` to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results queue_micro_task(() => { var root = anchor.getRootNode(); @@ -29,6 +15,8 @@ export function append_styles(anchor, css, is_custom_element) { ? /** @type {ShadowRoot} */ (root) : /** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head; + // Always querying the DOM is roughly the same perf as additionally checking for presence in a map first assuming + // that you'll get cache hits half of the time, so we just always query the dom for simplicity and code savings. if (!target.querySelector('#' + css.hash)) { const style = document.createElement('style'); style.id = css.hash; diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index a0483e9ea5..6195b2c561 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -1,5 +1,5 @@ import { createClassComponent } from '../../../../legacy/legacy-client.js'; -import { destroy_effect, render_effect } from '../../reactivity/effects.js'; +import { destroy_effect, effect_root, render_effect } from '../../reactivity/effects.js'; import { append } from '../template.js'; import { define_property, get_descriptor, object_keys } from '../../../shared/utils.js'; @@ -145,24 +145,26 @@ if (typeof HTMLElement === 'function') { }); // Reflect component props as attributes - this.$$me = render_effect(() => { - this.$$r = true; - for (const key of object_keys(this.$$c)) { - if (!this.$$p_d[key]?.reflect) continue; - this.$$d[key] = this.$$c[key]; - const attribute_value = get_custom_element_value( - key, - this.$$d[key], - this.$$p_d, - 'toAttribute' - ); - if (attribute_value == null) { - this.removeAttribute(this.$$p_d[key].attribute || key); - } else { - this.setAttribute(this.$$p_d[key].attribute || key, attribute_value); + this.$$me = effect_root(() => { + render_effect(() => { + this.$$r = true; + for (const key of object_keys(this.$$c)) { + if (!this.$$p_d[key]?.reflect) continue; + this.$$d[key] = this.$$c[key]; + const attribute_value = get_custom_element_value( + key, + this.$$d[key], + this.$$p_d, + 'toAttribute' + ); + if (attribute_value == null) { + this.removeAttribute(this.$$p_d[key].attribute || key); + } else { + this.setAttribute(this.$$p_d[key].attribute || key, attribute_value); + } } - } - this.$$r = false; + this.$$r = false; + }); }); for (const type in this.$$l) { @@ -196,7 +198,7 @@ if (typeof HTMLElement === 'function') { Promise.resolve().then(() => { if (!this.$$cn && this.$$c) { this.$$c.$destroy(); - destroy_effect(this.$$me); + this.$$me(); this.$$c = undefined; } }); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 66f11cda7c..d632840bfa 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -1,6 +1,14 @@ /** @import { Derived, Effect } from '#client' */ import { DEV } from 'esm-env'; -import { CLEAN, DERIVED, DESTROYED, DIRTY, MAYBE_DIRTY, UNOWNED } from '../constants.js'; +import { + CLEAN, + DERIVED, + DESTROYED, + DIRTY, + EFFECT_HAS_DERIVED, + MAYBE_DIRTY, + UNOWNED +} from '../constants.js'; import { active_reaction, active_effect, @@ -8,7 +16,8 @@ import { set_signal_status, skip_reaction, update_reaction, - increment_version + increment_version, + set_active_effect } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; @@ -23,7 +32,14 @@ import { inspect_effects, set_inspect_effects } from './sources.js'; /*#__NO_SIDE_EFFECTS__*/ export function derived(fn) { let flags = DERIVED | DIRTY; - if (active_effect === null) flags |= UNOWNED; + + if (active_effect === null) { + flags |= UNOWNED; + } else { + // Since deriveds are evaluated lazily, any effects created inside them are + // created too late to ensure that the parent effect is added to the tree + active_effect.f |= EFFECT_HAS_DERIVED; + } /** @type {Derived} */ const signal = { @@ -34,7 +50,8 @@ export function derived(fn) { fn, reactions: null, v: /** @type {V} */ (null), - version: 0 + version: 0, + parent: active_effect }; if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) { @@ -91,6 +108,9 @@ let stack = []; */ export function update_derived(derived) { var value; + var prev_active_effect = active_effect; + + set_active_effect(derived.parent); if (DEV) { let prev_inspect_effects = inspect_effects; @@ -105,12 +125,17 @@ export function update_derived(derived) { destroy_derived_children(derived); value = update_reaction(derived); } finally { + set_active_effect(prev_active_effect); set_inspect_effects(prev_inspect_effects); stack.pop(); } } else { - destroy_derived_children(derived); - value = update_reaction(derived); + try { + destroy_derived_children(derived); + value = update_reaction(derived); + } finally { + set_active_effect(prev_active_effect); + } } var status = diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index fa43f0afb2..b933c3d033 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -34,7 +34,8 @@ import { CLEAN, INSPECT_EFFECT, HEAD_EFFECT, - MAYBE_DIRTY + MAYBE_DIRTY, + EFFECT_HAS_DERIVED } from '../constants.js'; import { set } from './sources.js'; import * as e from '../errors.js'; @@ -138,7 +139,8 @@ function create_effect(type, fn, sync, push = true) { effect.deps === null && effect.first === null && effect.nodes_start === null && - effect.teardown === null; + effect.teardown === null && + (effect.f & EFFECT_HAS_DERIVED) === 0; if (!inert && !is_root && push) { if (parent_effect !== null) { @@ -203,7 +205,8 @@ export function user_effect(fn) { var context = /** @type {ComponentContext} */ (component_context); (context.e ??= []).push({ fn, - parent: active_effect + effect: active_effect, + reaction: active_reaction }); } else { var signal = effect(fn); diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index ecc3029580..8742905842 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -21,6 +21,7 @@ export interface Reaction extends Signal { fn: null | Function; /** Signals that this signal reads from */ deps: null | Value[]; + parent: Effect | null; } export interface Derived extends Value, Reaction { @@ -31,7 +32,6 @@ export interface Derived extends Value, Reaction { } export interface Effect extends Reaction { - parent: Effect | null; /** * Branch effects store their start/end nodes so that they can be * removed when the effect is destroyed, or moved when an `each` diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index f0bf696115..6a83ba7b2d 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -516,16 +516,11 @@ function flush_queued_root_effects(root_effects) { effect.f ^= EFFECT_HAS_DIRTY_CHILDREN; } - // When working with custom elements, the root effects might not have a root - if (effect.first === null && (effect.f & BRANCH_EFFECT) === 0) { - flush_queued_effects([effect]); - } else { - /** @type {Effect[]} */ - var collected_effects = []; + /** @type {Effect[]} */ + var collected_effects = []; - process_effects(effect, collected_effects); - flush_queued_effects(collected_effects); - } + process_effects(effect, collected_effects); + flush_queued_effects(collected_effects); } } finally { is_flushing_effect = previously_flushing_effect; @@ -1056,15 +1051,18 @@ export function pop(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.parent); + 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; diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 6d7065f1c8..7208ed7783 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -1,6 +1,6 @@ import type { Store } from '#shared'; import { STATE_SYMBOL } from './constants.js'; -import type { Effect, Source, Value } from './reactivity/types.js'; +import type { Effect, Source, Value, Reaction } from './reactivity/types.js'; type EventCallback = (event: Event) => boolean; export type EventCallbackMap = Record; @@ -15,7 +15,11 @@ export type ComponentContext = { /** context */ c: null | Map; /** deferred effects */ - e: null | Array<{ fn: () => void | (() => void); parent: null | Effect }>; + e: null | Array<{ + fn: () => void | (() => void); + effect: null | Effect; + reaction: null | Reaction; + }>; /** mounted */ m: boolean; /** diff --git a/packages/svelte/tests/migrate/samples/props-interface/input.svelte b/packages/svelte/tests/migrate/samples/props-interface/input.svelte new file mode 100644 index 0000000000..0f7ee86d99 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-interface/input.svelte @@ -0,0 +1,12 @@ + diff --git a/packages/svelte/tests/migrate/samples/props-interface/output.svelte b/packages/svelte/tests/migrate/samples/props-interface/output.svelte new file mode 100644 index 0000000000..e9d115cd5c --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-interface/output.svelte @@ -0,0 +1,13 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/Child.svelte b/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/Child.svelte index dac93751b8..9f2fc953cf 100644 --- a/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/Child.svelte +++ b/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/Child.svelte @@ -1,10 +1,13 @@

count: {count}

+ diff --git a/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/_config.js b/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/_config.js index 0f04e9659a..efc8db5870 100644 --- a/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/_config.js +++ b/packages/svelte/tests/runtime-browser/samples/mount-in-iframe/_config.js @@ -5,18 +5,20 @@ export default test({ async test({ target, assert }) { const button = target.querySelector('button'); const h1 = () => - /** @type {HTMLHeadingElement} */ ( + /** @type {NodeListOf} */ ( /** @type {Window} */ ( target.querySelector('iframe')?.contentWindow - ).document.querySelector('h1') + ).document.querySelectorAll('h1') ); - assert.equal(h1().textContent, 'count: 0'); - assert.equal(getComputedStyle(h1()).color, 'rgb(255, 0, 0)'); + assert.equal(h1()[0].textContent, 'count: 0'); + assert.equal(getComputedStyle(h1()[0]).color, 'rgb(255, 0, 0)'); + assert.equal(getComputedStyle(h1()[1]).color, 'rgb(0, 0, 255)'); flushSync(() => button?.click()); - assert.equal(h1().textContent, 'count: 1'); - assert.equal(getComputedStyle(h1()).color, 'rgb(255, 0, 0)'); + assert.equal(h1()[0].textContent, 'count: 1'); + assert.equal(getComputedStyle(h1()[0]).color, 'rgb(255, 0, 0)'); + assert.equal(getComputedStyle(h1()[1]).color, 'rgb(0, 0, 255)'); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js new file mode 100644 index 0000000000..34597fb66c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js @@ -0,0 +1,16 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: '', + + test({ assert, target }) { + const button = target.querySelector('button'); + + flushSync(() => button?.click()); + assert.htmlEqual(target.innerHTML, ''); + + flushSync(() => button?.click()); + assert.htmlEqual(target.innerHTML, ''); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte new file mode 100644 index 0000000000..bed341da91 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte @@ -0,0 +1,19 @@ + + +