8000 extract `onchange` callbacks from options by Rich-Harris · Pull Request #15579 · sveltejs/svelte · GitHub
[go: up one dir, main page]
Skip to content

extract onchange callbacks from options #15579

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.

Sign up for GitHub

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 10 commits into from
Mar 22, 2025
Merged

extract onchange callbacks from options #15579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
96e5edc
WIP
Rich-Harris Mar 21, 2025
ef1ffbe
extract onchange callbacks
Rich-Harris Mar 21, 2025
8fe6f32
const
Rich-Harris Mar 21, 2025
f0732a8
tweak
Rich-Harris Mar 21, 2025
c77dfde
docs
Rich-Harris Mar 21, 2025
8c08473
Merge branch 'state-onchange' into state-onchange-tweak
Rich-Harris Mar 21, 2025
7c387a8
Merge branch 'state-onchange' into state-onchange-tweak
Rich-Harris Mar 21, 2025
af3c5f4
fix: unwrap args in case of spread
paoloricciuti Mar 21, 2025
e4c7992
fix: revert unwrap args in case of spread
paoloricciuti Mar 21, 2025
976ff68
Merge branch 'state-onchange' into state-onchange-tweak
Rich-Harris Mar 22, 2025
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
12 changes: 8 additions & 4 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as b from '../../../../utils/builders.js';
import { regex_invalid_identifier_chars } from '../../../patterns.js';
import { get_rune } from '../../../scope.js';
import { should_proxy } from '../utils.js';
import { get_onchange } from './shared/state.js';

/**
* @param {ClassBody} node
Expand Down Expand Up @@ -117,13 +118,16 @@ export function ClassBody(node, context) {
);

if (field.kind === 'state' || field.kind === 'raw_state') {
let arg = definition.value.arguments[1];
let options = arg && /** @type {Expression} **/ (context.visit(arg, child_state));
const onchange = get_onchange(
/** @type {Expression} */ (definition.value.arguments[1]),
// @ts-ignore mismatch between Context and ComponentContext. TODO look into
context
);

value =
field.kind === 'state' && should_proxy(init, context.state.scope)
? b.call('$.assignable_proxy', init, options)
: b.call('$.state', init, options);
? b.call('$.assignable_proxy', init, onchange)
: b.call('$.state', init, onchange);
} else {
value = b.call('$.derived', field.kind === 'derived_by' ? init : b.thunk(init));
}
Expand Down
17 changes: 9 additions & 8 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as assert from '../../../../utils/assert.js';
import { get_rune } from '../../../scope.js';
import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js';
import { is_hoisted_function } from '../../utils.js';
import { get_onchange } from './shared/state.js';

/**
* @param {VariableDeclaration} node
Expand Down Expand Up @@ -117,34 +118,34 @@ export function VariableDeclaration(node, context) {

const args = /** @type {CallExpression} */ (init).arguments;
const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0;
let options =
args.length === 2 ? /** @type {Expression} */ (context.visit(args[1])) : undefined;

const onchange = get_onchange(/** @type {Expression} */ (args[1]), context);

if (rune === '$state' || rune === '$state.raw') {
/**
* @param {Identifier} id
* @param {Expression} value
* @param {Expression} [options]
* @param {Expression} [onchange]
*/
const create_state_declarator = (id, value, options) => {
const create_state_declarator = (id, value, onchange) => {
const binding = /** @type {Binding} */ (context.state.scope.get(id.name));
const proxied = rune === '$state' && should_proxy(value, context.state.scope);
const is_state = is_state_source(binding, context.state.analysis);

if (proxied) {
return b.call(is_state ? '$.assignable_proxy' : '$.proxy', value, options);
return b.call(is_state ? '$.assignable_proxy' : '$.proxy', value, onchange);
}

if (is_state) {
return b.call('$.state', value, options);
return b.call('$.state', value, onchange);
}

return value;
};

if (declarator.id.type === 'Identifier') {
declarations.push(
b.declarator(declarator.id, create_state_declarator(declarator.id, value, options))
b.declarator(declarator.id, create_state_declarator(declarator.id, value, onchange))
);
} else {
const tmp = context.state.scope.generate('tmp');
Expand All @@ -157,7 +158,7 @@ export function VariableDeclaration(node, context) {
return b.declarator(
path.node,
binding?.kind === 'state' || binding?.kind === 'raw_state'
? create_state_declarator(binding.node, value, options)
? create_state_declarator(binding.node, value, onchange)
: value
);
})
Expand Down
31 changes: 31 additions & 0 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/** @import { Expression, Property } from 'estree' */
/** @import { ComponentContext } from '../../types' */
import * as b from '../../../../../utils/builders.js';

/**
* Extract the `onchange` callback from the options passed to `$state`
* @param {Expression} options
* @param {ComponentContext} context
* @returns {Expression | undefined}
*/
export function get_onchange(options, context) {
if (!options) return;

if (options.type === 'ObjectExpression') {
const onchange = /** @type {Property | undefined} */ (
options.properties.find(
(property) =>
property.type === 'Property' &&
!property.computed &&
property.key.type === 'Identifier' &&
property.key.name === 'onchange'
)
);

if (!onchange) return;

return /** @type {Expression} */ (context.visit(onchange.value));
}

return b.member(/** @type {Expression} */ (context.visit(options)), 'onchange');
}
10 changes: 1 addition & 9 deletions packages/svelte/src/internal/client/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,7 @@ export {
user_effect,
user_pre_effect
} from './reactivity/effects.js';
export {
mutable_source,
mutate,
set,
state,
update,
update_pre,
get_options
} from './reactivity/sources.js';
export { mutable_source, mutate, set, state, update, update_pre } from './reactivity/sources.js';
export {
prop,
rest_props,
Expand Down
73 changes: 26 additions & 47 deletions packages/svelte/src/internal/client/proxy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,47 +28,33 @@ function identity(fn) {
return fn;
}

/**
* @param {ValueOptions | undefined} options
* @returns {ValueOptions | undefined}
*/
function clone_options(options) {
return options != null
? {
onchange: options.onchange
}
: undefined;
}

/** @type {ProxyMetadata | null} */
var parent_metadata = null;

/**
* @template T
* @param {T} value
* @param {ValueOptions} [_options]
* @param {() => void} [onchange]
* @param {Source<T>} [prev] dev mode only
* @returns {T}
*/
export function proxy(value, _options, prev) {
export function proxy(value, onchange, prev) {
// if non-proxyable, or is already a proxy, return `value`
if (typeof value !== 'object' || value === null) {
return value;
}

var options = clone_options(_options);

if (STATE_SYMBOL in value) {
// @ts-ignore
value[PROXY_ONCHANGE_SYMBOL](options?.onchange);
value[PROXY_ONCHANGE_SYMBOL](onchange);
return value;
}

if (options?.onchange) {
if (onchange) {
// if there's an onchange we actually store that but override the value
// to store every other onchange that new proxies might add
var onchanges = new Set([options.onchange]);
options.onchange = () => {
var onchanges = new Set([onchange]);
onchange = () => {
for (let onchange of onchanges) {
onchange();
}
Expand Down Expand Up @@ -116,10 +102,7 @@ export function proxy(value, _options, prev) {
if (is_proxied_array) {
// We need to create the length source eagerly to ensure that
// mutations to the array are properly synced with our proxy
sources.set(
'length',
source(/** @type {any[]} */ (value).length, clone_options(options), stack)
);
sources.set('length', source(/** @type {any[]} */ (value).length, onchange, stack));
}

/** @type {ProxyMetadata} */
Expand Down Expand Up @@ -165,12 +148,12 @@ export function proxy(value, _options, prev) {
var s = sources.get(prop);

if (s === undefined) {
s = with_parent(() => source(descriptor.value, clone_options(options), stack));
s = with_parent(() => source(descriptor.value, onchange, stack));
sources.set(prop, s);
} else {
set(
s,
with_parent(() => proxy(descriptor.value, options))
with_parent(() => proxy(descriptor.value, onchange))
);
}

Expand All @@ -184,7 +167,7 @@ export function proxy(value, _options, prev) {
if (prop in target) {
sources.set(
prop,
with_parent(() => source(UNINITIALIZED, clone_options(options), stack))
with_parent(() => source(UNINITIALIZED, onchange, stack))
);
}
} else {
Expand All @@ -201,7 +184,7 @@ export function proxy(value, _options, prev) {
// when we delete a property if the source is a proxy we remove the current onchange from
// the proxy `onchanges` so that it doesn't trigger it anymore
if (typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) {
s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true);
s.v[PROXY_ONCHANGE_SYMBOL](onchange, true);
}
set(s, UNINITIALIZED);
update_version(version);
Expand All @@ -227,18 +210,14 @@ export function proxy(value, _options, prev) {
// we either add or remove the passed in value
// to the onchanges array or we set every source onchange
// to the passed in value (if it's undefined it will make the chain stop)
if (options?.onchange != null && value && !remove) {
if (onchange != null && value && !remove) {
onchanges?.add?.(value);
} else if (options?.onchange != null && value) {
} else if (onchange != null && value) {
onchanges?.delete?.(value);
} else {
options = {
onchange: value
};
onchange = value;
for (let [, s] of sources) {
if (s.o) {
s.o.onchange = value;
}
s.o = value;
}
}
};
Expand All @@ -249,7 +228,7 @@ export function proxy(value, _options, prev) {

// create a source, but only if it's an own property and not a prototype property
if (s === undefined && (!exists || get_descriptor(target, prop)?.writable)) {
let opt = clone_options(options);
let opt = onchange;
s = with_parent(() =>
source(proxy(exists ? target[prop] : UNINITIALIZED, opt), opt, stack)
);
Expand Down Expand Up @@ -281,7 +260,7 @@ export function proxy(value, _options, prev) {

if (
is_proxied_array &&
options?.onchange != null &&
onchange != null &&
array_methods.includes(/** @type {string} */ (prop))
) {
return batch_onchange(v);
Expand Down Expand Up @@ -330,7 +309,7 @@ export function proxy(value, _options, prev) {
(active_effect !== null && (!has || get_descriptor(target, prop)?.writable))
) {
if (s === undefined) {
let opt = clone_options(options);
let opt = onchange;
s = with_parent(() => source(has ? proxy(target[prop], opt) : UNINITIALIZED, opt, stack));
sources.set(prop, s);
}
Expand Down Expand Up @@ -362,14 +341,14 @@ export function proxy(value, _options, prev) {
other_s.v !== null &&
STATE_SYMBOL in other_s.v
) {
other_s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true);
other_s.v[PROXY_ONCHANGE_SYMBOL](onchange, true);
}
set(other_s, UNINITIALIZED);
} else if (i in target) {
// If the item exists in the original, we need to create a uninitialized source,
// else a later read of the property would result in a source being created with
// the value of the original item at that index.
other_s = with_parent(() => source(UNINITIALIZED, clone_options(options), stack));
other_s = with_parent(() => source(UNINITIALIZED, onchange, stack));
sources.set(i + '', other_s);
}
}
Expand All @@ -381,7 +360,7 @@ export function proxy(value, _options, prev) {
// object property before writing to that property.
if (s === undefined) {
if (!has || get_descriptor(target, prop)?.writable) {
const opt = clone_options(options);
const opt = onchange;
s = with_parent(() => source(undefined, opt, stack));
set(
s,
Expand All @@ -394,11 +373,11 @@ export function proxy(value, _options, prev) {
// when we set a property if the source is a proxy we remove the current onchange from
// the proxy `onchanges` so that it doesn't trigger it anymore
if (typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) {
s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true);
s.v[PROXY_ONCHANGE_SYMBOL](onchange, true);
}
set(
s,
with_parent(() => proxy(value, clone_options(options)))
with_parent(() => proxy(value, onchange))
);
}
})();
Expand Down Expand Up @@ -464,11 +443,11 @@ export function proxy(value, _options, prev) {
/**
* @template T
* @param {T} value
* @param {ValueOptions} [options]
* @param {() => void} [onchange]
* @returns {Source<T>}
*/
export function assignable_proxy(value, options) {
return state(proxy(value, options), options);
export function assignable_proxy(value, onchange) {
return state(proxy(value, onchange), onchange);
}

/**
Expand Down
18 changes: 5 additions & 13 deletions packages/svelte/src/internal/client/reactivity/sources.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function batch_onchange(fn) {
/**
* @template V
* @param {V} v
* @param {ValueOptions} [o]
* @param {() => void} [o]
* @param {Error | null} [stack]
* @returns {Source<V>}
*/
Expand All @@ -102,18 +102,10 @@ export function source(v, o, stack) {
return signal;
}

/**
* @param {Source} source
* @returns {ValueOptions | undefined}
*/
export function get_options(source) {
return source.o;
}

/**
* @template V
* @param {V} v
* @param {ValueOptions} [o]
* @param {() => void} [o]
* @param {Error | null} [stack]
*/
export function state(v, o, stack) {
Expand Down Expand Up @@ -196,11 +188,11 @@ export function internal_set(source, value) {
if (!source.equals(value)) {
var old_value = source.v;

if (typeof old_value === 'object' && old_value != null && source.o?.onchange) {
if (typeof old_value === 'object' && old_value != null && source.o) {
// @ts-ignore
const remove = old_value[PROXY_ONCHANGE_SYMBOL];
if (remove && typeof remove === 'function') {
remove(source.o?.onchange, true);
remove(source.o, true);
}
}

Expand Down Expand Up @@ -257,7 +249,7 @@ export function internal_set(source, value) {
inspect_effects.clear();
}

var onchange = source.o?.onchange;
var onchange = source.o;
if (onchange) {
if (onchange_batch) {
onchange_batch.add(onchange);
Expand Down
Loading
0