Skip to content

chore: simplify props #16270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jul 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/new-trees-behave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

chore: simplify props
2 changes: 0 additions & 2 deletions packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ export const DESTROYED = 1 << 14;
export const EFFECT_RAN = 1 << 15;
/** 'Transparent' effects do not create a transition boundary */
export const EFFECT_TRANSPARENT = 1 << 16;
/** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */
export const LEGACY_DERIVED_PROP = 1 << 17;
export const INSPECT_EFFECT = 1 << 18;
export const HEAD_EFFECT = 1 << 19;
export const EFFECT_HAS_DERIVED = 1 << 20;
Expand Down
175 changes: 67 additions & 108 deletions packages/svelte/src/internal/client/reactivity/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ import {
PROPS_IS_UPDATED
} from '../../../constants.js';
import { get_descriptor, is_function } from '../../shared/utils.js';
import { mutable_source, set, source, update } from './sources.js';
import { set, source, update } from './sources.js';
import { derived, derived_safe_equal } from './deriveds.js';
import { get, captured_signals, untrack } from '../runtime.js';
import { safe_equals } from './equality.js';
import { get, untrack } from '../runtime.js';
import * as e from '../errors.js';
import { LEGACY_DERIVED_PROP, LEGACY_PROPS, STATE_SYMBOL } from '#client/constants';
import { LEGACY_PROPS, STATE_SYMBOL } from '#client/constants';
import { proxy } from '../proxy.js';
import { capture_store_binding } from './store.js';
import { legacy_mode_flag } from '../../flags/index.js';
Expand Down Expand Up @@ -260,89 +259,92 @@ function has_destroyed_component_ctx(current_value) {
* @returns {(() => V | ((arg: V) => V) | ((arg: V, mutation: boolean) => V))}
*/
export function prop(props, key, flags, fallback) {
var immutable = (flags & PROPS_IS_IMMUTABLE) !== 0;
var runes = !legacy_mode_flag || (flags & PROPS_IS_RUNES) !== 0;
var bindable = (flags & PROPS_IS_BINDABLE) !== 0;
var lazy = (flags & PROPS_IS_LAZY_INITIAL) !== 0;
var is_store_sub = false;
var prop_value;

if (bindable) {
[prop_value, is_store_sub] = capture_store_binding(() => /** @type {V} */ (props[key]));
} else {
prop_value = /** @type {V} */ (props[key]);
}

// Can be the case when someone does `mount(Component, props)` with `let props = $state({...})`
// or `createClassComponent(Component, props)`
var is_entry_props = STATE_SYMBOL in props || LEGACY_PROPS in props;

var setter =
(bindable &&
(get_descriptor(props, key)?.set ??
(is_entry_props && key in props && ((v) => (props[key] = v))))) ||
undefined;

var fallback_value = /** @type {V} */ (fallback);
var fallback_dirty = true;
var fallback_used = false;

var get_fallback = () => {
fallback_used = true;
if (fallback_dirty) {
fallback_dirty = false;
if (lazy) {
fallback_value = untrack(/** @type {() => V} */ (fallback));
} else {
fallback_value = /** @type {V} */ (fallback);
}

fallback_value = lazy
? untrack(/** @type {() => V} */ (fallback))
: /** @type {V} */ (fallback);
}

return fallback_value;
};

if (prop_value === undefined && fallback !== undefined) {
if (setter && runes) {
e.props_invalid_value(key);
}
/** @type {((v: V) => void) | undefined} */
var setter;

prop_value = get_fallback();
if (setter) setter(prop_value);
if (bindable) {
// Can be the case when someone does `mount(Component, props)` with `let props = $state({...})`
// or `createClassComponent(Component, props)`
var is_entry_props = STATE_SYMBOL in props || LEGACY_PROPS in props;

setter =
get_descriptor(props, key)?.set ??
(is_entry_props && key in props ? (v) => (props[key] = v) : undefined);
}

var initial_value;
var is_store_sub = false;

if (bindable) {
[initial_value, is_store_sub] = capture_store_binding(() => /** @type {V} */ (props[key]));
} else {
initial_value = /** @type {V} */ (props[key]);
}

if (initial_value === undefined && fallback !== undefined) {
initial_value = get_fallback();

if (setter) {
if (runes) e.props_invalid_value(key);
setter(initial_value);
}
}

/** @type {() => V} */
var getter;

if (runes) {
getter = () => {
var value = /** @type {V} */ (props[key]);
if (value === undefined) return get_fallback();
fallback_dirty = true;
fallback_used = false;
return value;
};
} else {
// Svelte 4 did not trigger updates when a primitive value was updated to the same value.
// Replicate that behavior through using a derived
var derived_getter = (immutable ? derived : derived_safe_equal)(
() => /** @type {V} */ (props[key])
);
derived_getter.f |= LEGACY_DERIVED_PROP;
getter = () => {
var value = get(derived_getter);
if (value !== undefined) fallback_value = /** @type {V} */ (undefined);
var value = /** @type {V} */ (props[key]);

if (value !== undefined) {
// in legacy mode, we don't revert to the fallback value
// if the prop goes from defined to undefined. The easiest
// way to model this is to make the fallback undefined
// as soon as the prop has a value
fallback_value = /** @type {V} */ (undefined);
}

return value === undefined ? fallback_value : value;
};
}

// easy mode — prop is never written to
if ((flags & PROPS_IS_UPDATED) === 0 && runes) {
// prop is never written to — we only need a getter
if ((flags & PROPS_IS_UPDATED) === 0) {
return getter;
}

// intermediate mode — prop is written to, but the parent component had
// `bind:foo` which means we can just call `$$props.foo = value` directly
// prop is written to, but the parent component had `bind:foo` which
// means we can just call `$$props.foo = value` directly
if (setter) {
var legacy_parent = props.$$legacy;

return function (/** @type {any} */ value, /** @type {boolean} */ mutation) {
if (arguments.length > 0) {
// We don't want to notify if the value was mutated and the parent is in runes mode.
Expand All @@ -352,82 +354,39 @@ export function prop(props, key, flags, fallback) {
if (!runes || !mutation || legacy_parent || is_store_sub) {
/** @type {Function} */ (setter)(mutation ? getter() : value);
}

return value;
} else {
return getter();
}

return getter();
};
}

// hard mode. this is where it gets ugly — the value in the child should
// synchronize with the parent, but it should also be possible to temporarily
// set the value to something else locally.
var from_child = false;
var was_from_child = false;

// The derived returns the current value. The underlying mutable
// source is written to from various places to persist this value.
var inner_current_value = mutable_source(prop_value);
var current_value = derived(() => {
var parent_value = getter();
var child_value = get(inner_current_value);

if (from_child) {
from_child = false;
was_from_child = true;
return child_value;
}

was_from_child = false;
return (inner_current_value.v = parent_value);
});

// Ensure we eagerly capture the initial value if it's bindable
if (bindable) {
get(current_value);
}
// prop is written to, but there's no binding, which means we
// create a derived that we can write to locally
Comment on lines +365 to +366
Copy link
Member

@dummdidumm dummdidumm Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found it strange that the "in Svelte 4 ..." logic above in the getter was removed - did some digging and turns out that #11577 was added back when we did return the getter above in legacy mode too. Now that logic is obsolete, but we should add a comment here explaining why we need it in legacy mode, too

Suggested change
// prop is written to, but there's no binding, which means we
// create a derived that we can write to locally
// Either prop is written to, but there's no binding, which means we
// create a derived that we can write to locally.
// Or we are in legacy mode where we always create a derived to replicate that
// Svelte 4 did not trigger updates when a primitive value was updated to the same value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out we actually don't need the runes/legacy check when the prop isn't written to — 3933328

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do need it, the test hides the bug because it's using accessors by default. PR with fix incoming

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var d = ((flags & PROPS_IS_IMMUTABLE) !== 0 ? derived : derived_safe_equal)(getter);

if (!immutable) current_value.equals = safe_equals;
// Capture the initial value if it's bindable
if (bindable) get(d);

return function (/** @type {any} */ value, /** @type {boolean} */ mutation) {
// legacy nonsense — need to ensure the source is invalidated when necessary
// also needed for when handling inspect logic so we can inspect the correct source signal
if (captured_signals !== null) {
// set this so that we don't reset to the parent value if `d`
// is invalidated because of `invalidate_inner_signals` (rather
// than because the parent or child value changed)
from_child = was_from_child;
// invoke getters so that signals are picked up by `invalidate_inner_signals`
getter();
get(inner_current_value);
}

if (arguments.length > 0) {
const new_value = mutation ? get(current_value) : runes && bindable ? proxy(value) : value;

if (!current_value.equals(new_value)) {
from_child = true;
set(inner_current_value, new_value);
// To ensure the fallback value is consistent when used with proxies, we
// update the local fallback_value, but only if the fallback is actively used
if (fallback_used && fallback_value !== undefined) {
fallback_value = new_value;
}
const new_value = mutation ? get(d) : runes && bindable ? proxy(value) : value;

if (has_destroyed_component_ctx(current_value)) {
return value;
}
set(d, new_value);

untrack(() => get(current_value)); // force a synchronisation immediately
if (fallback_value !== undefined) {
fallback_value = new_value;
}

return value;
}

if (has_destroyed_component_ctx(current_value)) {
return current_value.v;
// TODO is this still necessary post-#16263?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure yes because things not read in a teardown can still cause bugs as seen in #16072 - we might need to make props signals after all, gonna investigate that soon

if (has_destroyed_component_ctx(d)) {
return d.v;
}

return get(current_value);
return get(d);
};
}
13 changes: 1 addition & 12 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
STATE_SYMBOL,
BLOCK_EFFECT,
ROOT_EFFECT,
LEGACY_DERIVED_PROP,
DISCONNECTED,
EFFECT_IS_UPDATING,
STALE_REACTION
Expand Down Expand Up @@ -863,17 +862,7 @@ export function invalidate_inner_signals(fn) {
var captured = capture_signals(() => untrack(fn));

for (var signal of captured) {
// Go one level up because derived signals created as part of props in legacy mode
if ((signal.f & LEGACY_DERIVED_PROP) !== 0) {
for (const dep of /** @type {Derived} */ (signal).deps || []) {
if ((dep.f & DERIVED) === 0) {
// Use internal_set instead of set here and below to avoid mutation validation
internal_set(dep, dep.v);
}
}
} else {
internal_set(signal, signal.v);
}
internal_set(signal, signal.v);
}
}

Expand Down