From 56ecdbaeaca2eddcb1527e27385f045a8025dd00 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Apr 2024 14:21:18 -0400 Subject: [PATCH 1/5] DRY out snapshot code --- .../svelte/src/internal/client/dev/inspect.js | 45 +------------- packages/svelte/src/internal/client/index.js | 3 +- packages/svelte/src/internal/client/proxy.js | 58 ------------------- .../internal/client/reactivity/snapshot.js | 46 +++++++++++++++ .../svelte/src/internal/client/runtime.js | 2 +- .../samples/inspect-derived-2/_config.js | 21 ++++--- .../samples/inspect-nested-state/_config.js | 26 ++++----- .../samples/inspect-nested-state/main.svelte | 2 +- .../samples/inspect-new-property/_config.js | 19 +++--- 9 files changed, 85 insertions(+), 137 deletions(-) create mode 100644 packages/svelte/src/internal/client/reactivity/snapshot.js diff --git a/packages/svelte/src/internal/client/dev/inspect.js b/packages/svelte/src/internal/client/dev/inspect.js index 8e3c11dd63e5..bc3c4edca80e 100644 --- a/packages/svelte/src/internal/client/dev/inspect.js +++ b/packages/svelte/src/internal/client/dev/inspect.js @@ -1,7 +1,6 @@ -import { snapshot } from '../proxy.js'; +import { snapshot } from '../reactivity/snapshot.js'; import { render_effect } from '../reactivity/effects.js'; import { current_effect, deep_read } from '../runtime.js'; -import { array_prototype, get_prototype_of, object_prototype } from '../utils.js'; /** @type {Function | null} */ export let inspect_fn = null; @@ -32,7 +31,7 @@ export function inspect(get_value, inspector = console.log) { // calling `inspector` directly inside the effect, so that // we get useful stack traces var fn = () => { - const value = deep_snapshot(get_value()); + const value = snapshot(get_value()); inspector(initial ? 'init' : 'update', ...value); }; @@ -56,43 +55,3 @@ export function inspect(get_value, inspector = console.log) { }; }); } - -/** - * Like `snapshot`, but recursively traverses into normal arrays/objects to find potential states in them. - * @param {any} value - * @param {Map} visited - * @returns {any} - */ -function deep_snapshot(value, visited = new Map()) { - if (typeof value === 'object' && value !== null && !visited.has(value)) { - const unstated = snapshot(value); - - if (unstated !== value) { - visited.set(value, unstated); - return unstated; - } - - const prototype = get_prototype_of(value); - - // Only deeply snapshot plain objects and arrays - if (prototype === object_prototype || prototype === array_prototype) { - let contains_unstated = false; - /** @type {any} */ - const nested_unstated = Array.isArray(value) ? [] : {}; - - for (let key in value) { - const result = deep_snapshot(value[key], visited); - nested_unstated[key] = result; - if (result !== value[key]) { - contains_unstated = true; - } - } - - visited.set(value, contains_unstated ? nested_unstated : value); - } else { - visited.set(value, value); - } - } - - return visited.get(value) ?? value; -} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 2362698ab183..0e392a74ae39 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -88,6 +88,7 @@ export { update_pre_prop, update_prop } from './reactivity/props.js'; +export { snapshot } from './reactivity/snapshot.js'; export { invalidate_store, mutate_store, @@ -128,7 +129,7 @@ export { validate_store } from './validate.js'; export { raf } from './timing.js'; -export { proxy, snapshot } from './proxy.js'; +export { proxy } from './proxy.js'; export { create_custom_element } from './dom/elements/custom-element.js'; export { child, diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 136458c99d35..1d4c6ed245f8 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -10,7 +10,6 @@ import { array_prototype, define_property, get_descriptor, - get_descriptors, get_prototype_of, is_array, is_frozen, @@ -87,63 +86,6 @@ export function proxy(value, immutable = true, parent = null) { return value; } -/** - * @template {import('#client').ProxyStateObject} T - * @param {T} value - * @param {Map>} already_unwrapped - * @returns {Record} - */ -function unwrap(value, already_unwrapped) { - if (typeof value === 'object' && value != null && STATE_SYMBOL in value) { - const unwrapped = already_unwrapped.get(value); - if (unwrapped !== undefined) { - return unwrapped; - } - - if (is_array(value)) { - /** @type {Record} */ - const array = []; - already_unwrapped.set(value, array); - for (const element of value) { - array.push(unwrap(element, already_unwrapped)); - } - return array; - } else { - /** @type {Record} */ - const obj = {}; - const keys = Reflect.ownKeys(value); - const descriptors = get_descriptors(value); - already_unwrapped.set(value, obj); - - for (const key of keys) { - if (key === STATE_SYMBOL) continue; - if (descriptors[key].get) { - define_property(obj, key, descriptors[key]); - } else { - /** @type {T} */ - const property = value[key]; - obj[key] = unwrap(property, already_unwrapped); - } - } - - return obj; - } - } - - return value; -} - -/** - * @template T - * @param {T} value - * @returns {T} - */ -export function snapshot(value) { - return /** @type {T} */ ( - unwrap(/** @type {import('#client').ProxyStateObject} */ (value), new Map()) - ); -} - /** * @param {import('#client').Source} signal * @param {1 | -1} [d] diff --git a/packages/svelte/src/internal/client/reactivity/snapshot.js b/packages/svelte/src/internal/client/reactivity/snapshot.js new file mode 100644 index 000000000000..2fb55b27088d --- /dev/null +++ b/packages/svelte/src/internal/client/reactivity/snapshot.js @@ -0,0 +1,46 @@ +import { STATE_SYMBOL } from '../constants.js'; +import { is_array } from '../utils.js'; + +/** + * @template {any} T + * @param {T} value + * @param {Map} values + * @returns {T} + */ +export function snapshot(value, values = new Map()) { + if (typeof value !== 'object' || value === null) { + return value; + } + + if (STATE_SYMBOL in value) { + var unwrapped = /** @type {T} */ (values.get(value)); + if (unwrapped !== undefined) { + return unwrapped; + } + + if (is_array(value)) { + var length = value.length; + var array = Array(length); + + values.set(value, array); + + for (var i = 0; i < length; i += 1) { + array[i] = snapshot(value[i], values); + } + + return /** @type {T} */ (array); + } + + /** @type {Record} */ + var obj = {}; + values.set(value, obj); + + for (var [k, v] of Object.entries(value)) { + obj[k] = snapshot(v, values); + } + + return /** @type {T} */ (obj); + } + + return value; +} diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 88b7110b9968..6fd0e4c4763b 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1,6 +1,6 @@ import { DEV } from 'esm-env'; import { get_descriptors, get_prototype_of, is_frozen, object_freeze } from './utils.js'; -import { snapshot } from './proxy.js'; +import { snapshot } from './reactivity/snapshot.js'; import { destroy_effect, effect, execute_effect_teardown } from './reactivity/effects.js'; import { EFFECT, diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js index 0e6cf8588423..68774f1e67f2 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js @@ -25,13 +25,6 @@ export default test({ console.log = original_log; }, async test({ assert, target }) { - const button = target.querySelector('button'); - - flushSync(() => { - button?.click(); - }); - - assert.htmlEqual(target.innerHTML, `\n1`); assert.deepEqual(log, [ 'init', { @@ -40,7 +33,19 @@ export default test({ list: [] }, derived: [] - }, + } + ]); + + log.length = 0; + + const button = target.querySelector('button'); + + flushSync(() => { + button?.click(); + }); + + assert.htmlEqual(target.innerHTML, `\n1`); + assert.deepEqual(log, [ 'update', { data: { diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/_config.js index 32c9e9829613..d6b15d84ef26 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/_config.js @@ -1,40 +1,34 @@ import { test } from '../../test'; -/** - * @type {any[]} - */ +/** @type {any[]} */ let log; -/** - * @type {typeof console.log}} - */ -let original_log; + +let original_log = console.log; export default test({ compileOptions: { dev: true }, + before_test() { log = []; - original_log = console.log; console.log = (...v) => { log.push(...v); }; }, + after_test() { console.log = original_log; }, + async test({ assert, target }) { + assert.deepEqual(log, ['init', { x: { count: 0 } }, [{ count: 0 }]]); + log.length = 0; + const [b1] = target.querySelectorAll('button'); b1.click(); await Promise.resolve(); - assert.deepEqual(log, [ - 'init', - { x: { count: 0 } }, - [{ count: 0 }], - 'update', - { x: { count: 1 } }, - [{ count: 1 }] - ]); + assert.deepEqual(log, ['update', { x: { count: 1 } }, [{ count: 1 }]]); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/main.svelte index 68a171b80d5e..a9b838a96f94 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/inspect-nested-state/main.svelte @@ -1,5 +1,5 @@ diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js index 5e5150bf0d15..91f988358f6e 100644 --- a/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/inspect-new-property/_config.js @@ -1,33 +1,34 @@ import { test } from '../../test'; -/** - * @type {any[]} - */ +/** @type {any[]} */ let log; -/** - * @type {typeof console.log}} - */ -let original_log; + +let original_log = console.log; export default test({ compileOptions: { dev: true }, + before_test() { log = []; - original_log = console.log; console.log = (...v) => { log.push(...v); }; }, + after_test() { console.log = original_log; }, + async test({ assert, target }) { + assert.deepEqual(log, ['init', {}, 'init', []]); + log.length = 0; + const [btn] = target.querySelectorAll('button'); btn.click(); await Promise.resolve(); - assert.deepEqual(log, ['init', {}, 'init', [], 'update', { x: 'hello' }, 'update', ['hello']]); + assert.deepEqual(log, ['update', { x: 'hello' }, 'update', ['hello']]); } }); From ff2e489883d71b6954602ca6c1cd8948b0ca3698 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Apr 2024 16:51:36 -0400 Subject: [PATCH 2/5] deep snapshots --- .../svelte/src/internal/client/dev/inspect.js | 2 +- .../src/internal/client/reactivity/snapshot.js | 16 +++++++++++----- packages/svelte/src/internal/client/runtime.js | 5 ++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/inspect.js b/packages/svelte/src/internal/client/dev/inspect.js index bc3c4edca80e..73ae94e239c9 100644 --- a/packages/svelte/src/internal/client/dev/inspect.js +++ b/packages/svelte/src/internal/client/dev/inspect.js @@ -31,7 +31,7 @@ export function inspect(get_value, inspector = console.log) { // calling `inspector` directly inside the effect, so that // we get useful stack traces var fn = () => { - const value = snapshot(get_value()); + const value = snapshot(get_value(), true); inspector(initial ? 'init' : 'update', ...value); }; diff --git a/packages/svelte/src/internal/client/reactivity/snapshot.js b/packages/svelte/src/internal/client/reactivity/snapshot.js index 2fb55b27088d..009838e7bd45 100644 --- a/packages/svelte/src/internal/client/reactivity/snapshot.js +++ b/packages/svelte/src/internal/client/reactivity/snapshot.js @@ -1,18 +1,24 @@ import { STATE_SYMBOL } from '../constants.js'; -import { is_array } from '../utils.js'; +import { array_prototype, get_prototype_of, is_array, object_prototype } from '../utils.js'; /** * @template {any} T * @param {T} value + * @param {boolean} deep * @param {Map} values * @returns {T} */ -export function snapshot(value, values = new Map()) { +export function snapshot(value, deep = false, values = new Map()) { if (typeof value !== 'object' || value === null) { return value; } - if (STATE_SYMBOL in value) { + var proto = get_prototype_of(value); + + if ( + (proto === object_prototype || proto === array_prototype) && + (deep || STATE_SYMBOL in value) + ) { var unwrapped = /** @type {T} */ (values.get(value)); if (unwrapped !== undefined) { return unwrapped; @@ -25,7 +31,7 @@ export function snapshot(value, values = new Map()) { values.set(value, array); for (var i = 0; i < length; i += 1) { - array[i] = snapshot(value[i], values); + array[i] = snapshot(value[i], deep, values); } return /** @type {T} */ (array); @@ -36,7 +42,7 @@ export function snapshot(value, values = new Map()) { values.set(value, obj); for (var [k, v] of Object.entries(value)) { - obj[k] = snapshot(v, values); + obj[k] = snapshot(v, deep, values); } return /** @type {T} */ (obj); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 6fd0e4c4763b..1a3ae8e581e4 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -125,9 +125,8 @@ export function is_runes() { */ export function batch_inspect(target, prop, receiver) { const value = Reflect.get(target, prop, receiver); - /** - * @this {any} - */ + + /** @this {any} */ return function () { const previously_batching_effect = is_batching_effect; is_batching_effect = true; From 05987bc71443675a3862dae6bba60396c3dca41b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Apr 2024 16:52:33 -0400 Subject: [PATCH 3/5] note to self --- packages/svelte/src/internal/client/dev/inspect.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/svelte/src/internal/client/dev/inspect.js b/packages/svelte/src/internal/client/dev/inspect.js index 73ae94e239c9..82caa0174855 100644 --- a/packages/svelte/src/internal/client/dev/inspect.js +++ b/packages/svelte/src/internal/client/dev/inspect.js @@ -37,6 +37,9 @@ export function inspect(get_value, inspector = console.log) { render_effect(() => { inspect_fn = fn; + // TODO ideally we'd use `snapshot` here instead of `deep_read`, and pass + // the result to `fn` on the initial run, but it doesn't work because + // of some weird (and possibly buggy?) behaviour around `batch_inspect` deep_read(get_value()); inspect_fn = null; From ee702d153fd996a7dd7534405b31b335ce454e10 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Apr 2024 17:04:01 -0400 Subject: [PATCH 4/5] snapshot maps --- .../svelte/src/internal/client/constants.js | 1 + .../src/internal/client/reactivity/snapshot.js | 17 +++++++++++------ packages/svelte/src/internal/client/runtime.js | 10 +++++++++- packages/svelte/src/reactivity/map.js | 13 +++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index b9953b2ec6c2..d96c15be00a9 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -16,3 +16,4 @@ export const EFFECT_RAN = 1 << 13; export const EFFECT_TRANSPARENT = 1 << 14; export const STATE_SYMBOL = Symbol('$state'); +export const STATE_SNAPSHOT_SYMBOL = Symbol('$state.snapshot'); diff --git a/packages/svelte/src/internal/client/reactivity/snapshot.js b/packages/svelte/src/internal/client/reactivity/snapshot.js index 009838e7bd45..10532af7e414 100644 --- a/packages/svelte/src/internal/client/reactivity/snapshot.js +++ b/packages/svelte/src/internal/client/reactivity/snapshot.js @@ -1,4 +1,4 @@ -import { STATE_SYMBOL } from '../constants.js'; +import { STATE_SNAPSHOT_SYMBOL, STATE_SYMBOL } from '../constants.js'; import { array_prototype, get_prototype_of, is_array, object_prototype } from '../utils.js'; /** @@ -13,17 +13,22 @@ export function snapshot(value, deep = false, values = new Map()) { return value; } + var unwrapped = /** @type {T} */ (values.get(value)); + if (unwrapped !== undefined) { + return unwrapped; + } + + if (STATE_SNAPSHOT_SYMBOL in value) { + // @ts-expect-error + return value[STATE_SNAPSHOT_SYMBOL](deep); + } + var proto = get_prototype_of(value); if ( (proto === object_prototype || proto === array_prototype) && (deep || STATE_SYMBOL in value) ) { - var unwrapped = /** @type {T} */ (values.get(value)); - if (unwrapped !== undefined) { - return unwrapped; - } - if (is_array(value)) { var length = value.length; var array = Array(length); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 1a3ae8e581e4..96747c799aca 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -15,7 +15,8 @@ import { BRANCH_EFFECT, STATE_SYMBOL, BLOCK_EFFECT, - ROOT_EFFECT + ROOT_EFFECT, + STATE_SNAPSHOT_SYMBOL } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; @@ -1137,6 +1138,12 @@ export function deep_read(value, visited = new Set()) { !visited.has(value) ) { visited.add(value); + + if (STATE_SNAPSHOT_SYMBOL in value) { + value[STATE_SNAPSHOT_SYMBOL](true); + return; + } + for (let key in value) { try { deep_read(value[key], visited); @@ -1144,6 +1151,7 @@ export function deep_read(value, visited = new Set()) { // continue } } + const proto = get_prototype_of(value); if ( proto !== Object.prototype && diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index b2b3ab605b64..c1aea58fd840 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -3,6 +3,8 @@ import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; import { UNINITIALIZED } from '../constants.js'; import { map } from './utils.js'; +import { STATE_SNAPSHOT_SYMBOL } from '../internal/client/constants.js'; +import { snapshot } from '../internal/client/reactivity/snapshot.js'; /** * @template K @@ -155,4 +157,15 @@ export class ReactiveMap extends Map { get size() { return get(this.#size); } + + /** @param {boolean} deep */ + [STATE_SNAPSHOT_SYMBOL](deep) { + return new Map( + map( + this.#sources.entries(), + ([key, source]) => /** @type {[K, V]} */ ([key, snapshot(get(source), deep)]), + 'Map Iterator' + ) + ); + } } From 1602ae62bdcdfb253191c7375a2bb1972a016d3e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Apr 2024 17:19:49 -0400 Subject: [PATCH 5/5] Set, Date, URL --- packages/svelte/src/reactivity/date.js | 5 +++++ packages/svelte/src/reactivity/set.js | 7 +++++++ packages/svelte/src/reactivity/url.js | 5 +++++ 3 files changed, 17 insertions(+) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index cf43dfe4ab43..0fb23f4420dd 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -1,3 +1,4 @@ +import { STATE_SNAPSHOT_SYMBOL } from '../internal/client/constants.js'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; @@ -99,4 +100,8 @@ export class ReactiveDate extends Date { super(...values); this.#init(); } + + [STATE_SNAPSHOT_SYMBOL]() { + return new Date(get(this.#raw_time)); + } } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 1855090cdfa5..99b180c5ed67 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -2,6 +2,8 @@ import { DEV } from 'esm-env'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; import { map } from './utils.js'; +import { STATE_SNAPSHOT_SYMBOL } from '../internal/client/constants.js'; +import { snapshot } from '../internal/client/reactivity/snapshot.js'; var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf']; var set_like_methods = ['difference', 'intersection', 'symmetricDifference', 'union']; @@ -149,4 +151,9 @@ export class ReactiveSet extends Set { get size() { return get(this.#size); } + + /** @param {boolean} deep */ + [STATE_SNAPSHOT_SYMBOL](deep) { + return new Set(map(this.keys(), (key) => snapshot(key, deep), 'Set Iterator')); + } } diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 3f11a4af37c7..89d4114da01e 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -1,3 +1,4 @@ +import { STATE_SNAPSHOT_SYMBOL } from '../internal/client/constants.js'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; @@ -150,6 +151,10 @@ export class ReactiveURL extends URL { toJSON() { return this.href; } + + [STATE_SNAPSHOT_SYMBOL]() { + return new URL(this.href); + } } export class ReactiveURLSearchParams extends URLSearchParams {