From c5fdd26bbcace5bad9f811d9ac77d47e48da3a32 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 8 Jul 2024 16:06:50 +0100 Subject: [PATCH 1/7] fix: resolve legacy component props equality for mutations --- .changeset/six-chicken-kneel.md | 5 ++ packages/svelte/src/legacy/legacy-client.js | 46 ++++++++++++++++++- .../samples/component-props-each/_config.js | 33 +++++++++++++ .../samples/component-props-each/main.svelte | 7 +++ 4 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 .changeset/six-chicken-kneel.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte diff --git a/.changeset/six-chicken-kneel.md b/.changeset/six-chicken-kneel.md new file mode 100644 index 000000000000..f7fcbd14c69f --- /dev/null +++ b/.changeset/six-chicken-kneel.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: resolve legacy component props equality for mutations diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js index d2eba5be9cf7..d3cb3cfbd1f8 100644 --- a/packages/svelte/src/legacy/legacy-client.js +++ b/packages/svelte/src/legacy/legacy-client.js @@ -1,8 +1,9 @@ /** @import { ComponentConstructorOptions, ComponentType, SvelteComponent, Component } from 'svelte' */ -import { proxy } from '../internal/client/proxy.js'; +import { mutable_source, get, set } from 'svelte/internal/client'; import { user_pre_effect } from '../internal/client/reactivity/effects.js'; import { hydrate, mount, unmount } from '../internal/client/render.js'; import { define_property } from '../internal/client/utils.js'; +import { safe_not_equal } from '../internal/client/reactivity/equality.js'; /** * Takes the same options as a Svelte 4 component and the component function and returns a Svelte 4 compatible component. @@ -69,10 +70,51 @@ class Svelte4Component { * }} options */ constructor(options) { + var sources = new Map(); + var add_source = (/** @type {string | symbol} */ key) => { + var s = mutable_source(0); + sources.set(key, s); + return s; + }; // Using proxy state here isn't completely mirroring the Svelte 4 behavior, because mutations to a property // cause fine-grained updates to only the places where that property is used, and not the entire property. // Reactive statements and actions (the things where this matters) are handling this properly regardless, so it should be fine in practise. - const props = proxy({ ...(options.props || {}), $$events: {} }, false); + const props = new Proxy( + { ...(options.props || {}), $$events: {} }, + { + get(target, prop, receiver) { + var value = Reflect.get(target, prop, receiver); + var s = sources.get(prop); + if (s === undefined) { + s = add_source(prop); + } + get(s); + return value; + }, + has(target, prop) { + var value = Reflect.has(target, prop); + var s = sources.get(prop); + if (s !== undefined) { + get(s); + } + return value; + }, + set(target, prop, value) { + var s = sources.get(prop); + // @ts-ignore + var prev_value = target[prop]; + if (s === undefined) { + s = add_source(prop); + } else if (safe_not_equal(prev_value, value)) { + // Increment version + set(s, s.v + 1); + } + // @ts-ignore + target[prop] = value; + return true; + } + } + ); this.#instance = (options.hydrate ? hydrate : mount)(options.component, { target: options.target, props, diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js new file mode 100644 index 000000000000..a29854fc4e4d --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js @@ -0,0 +1,33 @@ +import { test } from '../../test'; + +const data = { + items: [1, 1] +}; + +export default test({ + get props() { + data.items = [1, 1]; + + return { + data + }; + }, + + async test({ assert, component, target }) { + assert.htmlEqual( + target.innerHTML, + `
1
1
` + ); + + data.items = [2, 2]; + + await component.$set({ + data + }); + + assert.htmlEqual( + target.innerHTML, + `
2
2
` + ); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte b/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte new file mode 100644 index 000000000000..3ef77483546d --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte @@ -0,0 +1,7 @@ + + +{#each data.items as item} +
{item}
+{/each} From 2697c0845f7b3f2f66c9327a238040298cd8c86a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 8 Jul 2024 16:07:53 +0100 Subject: [PATCH 2/7] lint --- .../samples/component-props-each/_config.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js index a29854fc4e4d..35bf4df6e169 100644 --- a/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js @@ -14,10 +14,7 @@ export default test({ }, async test({ assert, component, target }) { - assert.htmlEqual( - target.innerHTML, - `
1
1
` - ); + assert.htmlEqual(target.innerHTML, `
1
1
`); data.items = [2, 2]; @@ -25,9 +22,6 @@ export default test({ data }); - assert.htmlEqual( - target.innerHTML, - `
2
2
` - ); + assert.htmlEqual(target.innerHTML, `
2
2
`); } }); From a2478480f4a0a756f52f10e4356fe81395c33aa1 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 20:52:44 +0200 Subject: [PATCH 3/7] Update packages/svelte/src/legacy/legacy-client.js --- packages/svelte/src/legacy/legacy-client.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js index d3cb3cfbd1f8..0cb96760c5f5 100644 --- a/packages/svelte/src/legacy/legacy-client.js +++ b/packages/svelte/src/legacy/legacy-client.js @@ -76,9 +76,9 @@ class Svelte4Component { sources.set(key, s); return s; }; - // Using proxy state here isn't completely mirroring the Svelte 4 behavior, because mutations to a property - // cause fine-grained updates to only the places where that property is used, and not the entire property. - // Reactive statements and actions (the things where this matters) are handling this properly regardless, so it should be fine in practise. + // Replicate coarse-grained props through a proxy that has a version source for + // each property, which is increment on updates to the property itself. Do not + // use our $state proxy because that one has fine-grained reactivity. const props = new Proxy( { ...(options.props || {}), $$events: {} }, { From 397cccf0601bb883e2ea10435d140e0ccd0ace17 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Jul 2024 15:09:54 -0400 Subject: [PATCH 4/7] simplify test --- .../samples/component-props-each/_config.js | 17 +++++++---------- .../samples/component-props-each/main.svelte | 4 +--- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js index 35bf4df6e169..91d0aaa0d3c1 100644 --- a/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js @@ -1,27 +1,24 @@ import { test } from '../../test'; const data = { - items: [1, 1] + message: 'hello' }; export default test({ get props() { - data.items = [1, 1]; + data.message = 'hello'; return { data }; }, - async test({ assert, component, target }) { - assert.htmlEqual(target.innerHTML, `
1
1
`); - - data.items = [2, 2]; + html: '

hello

', - await component.$set({ - data - }); + async test({ assert, component, target }) { + data.message = 'goodbye'; + await component.$set({ data }); - assert.htmlEqual(target.innerHTML, `
2
2
`); + assert.htmlEqual(target.innerHTML, '

goodbye

'); } }); diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte b/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte index 3ef77483546d..30195a083646 100644 --- a/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte +++ b/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte @@ -2,6 +2,4 @@ export let data; -{#each data.items as item} -
{item}
-{/each} +

{data.message}

From e049e85c43238c4a09c6492419c787a047743097 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Jul 2024 15:10:02 -0400 Subject: [PATCH 5/7] rename test --- .../{component-props-each => component-props-mutated}/_config.js | 0 .../{component-props-each => component-props-mutated}/main.svelte | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename packages/svelte/tests/runtime-legacy/samples/{component-props-each => component-props-mutated}/_config.js (100%) rename packages/svelte/tests/runtime-legacy/samples/{component-props-each => component-props-mutated}/main.svelte (100%) diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/component-props-mutated/_config.js similarity index 100% rename from packages/svelte/tests/runtime-legacy/samples/component-props-each/_config.js rename to packages/svelte/tests/runtime-legacy/samples/component-props-mutated/_config.js diff --git a/packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte b/packages/svelte/tests/runtime-legacy/samples/component-props-mutated/main.svelte similarity index 100% rename from packages/svelte/tests/runtime-legacy/samples/component-props-each/main.svelte rename to packages/svelte/tests/runtime-legacy/samples/component-props-mutated/main.svelte From c056f887780404bca2ea664c286b6f6768929fbf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Jul 2024 15:18:44 -0400 Subject: [PATCH 6/7] make all proxies immutable --- packages/svelte/src/internal/client/proxy.js | 15 ++++++--------- packages/svelte/src/internal/client/types.d.ts | 2 -- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 78d835dec420..2aebc35d6d97 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -11,7 +11,7 @@ import { object_prototype } from './utils.js'; import { check_ownership, widen_ownership } from './dev/ownership.js'; -import { mutable_source, source, set } from './reactivity/sources.js'; +import { source, set } from './reactivity/sources.js'; import { STATE_FROZEN_SYMBOL, STATE_SYMBOL } from './constants.js'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; @@ -19,7 +19,7 @@ import * as e from './errors.js'; /** * @template T * @param {T} value - * @param {boolean} [immutable] + * @param {true} [immutable] * @param {import('#client').ProxyMetadata | null} [parent] * @param {import('#client').Source} [prev] dev mode only * @returns {import('#client').ProxyStateObject | T} @@ -59,7 +59,6 @@ export function proxy(value, immutable = true, parent = null, prev) { s: new Map(), v: source(0), a: is_array(value), - i: immutable, p: proxy, t: value }), @@ -169,7 +168,7 @@ const state_proxy_handler = { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); - if (s !== undefined) set(s, proxy(descriptor.value, metadata.i, metadata)); + if (s !== undefined) set(s, proxy(descriptor.value, true, metadata)); } return Reflect.defineProperty(target, prop, descriptor); @@ -215,7 +214,7 @@ const state_proxy_handler = { // create a source, but only if it's an own property and not a prototype property if (s === undefined && (!(prop in target) || get_descriptor(target, prop)?.writable)) { - s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata)); + s = source(proxy(target[prop], true, metadata)); metadata.s.set(prop, s); } @@ -256,9 +255,7 @@ const state_proxy_handler = { (current_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = (metadata.i ? source : mutable_source)( - has ? proxy(target[prop], metadata.i, metadata) : UNINITIALIZED - ); + s = source(has ? proxy(target[prop], true, metadata) : UNINITIALIZED); metadata.s.set(prop, s); } const value = get(s); @@ -283,7 +280,7 @@ const state_proxy_handler = { s = metadata.s.get(prop); } if (s !== undefined) { - set(s, proxy(value, metadata.i, metadata)); + set(s, proxy(value, true, metadata)); } const is_array = metadata.a; const not_has = !(prop in target); diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 0d2d5a29c2c4..78b1d8d6fde5 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -180,8 +180,6 @@ export interface ProxyMetadata> { v: Source; /** `true` if the proxified object is an array */ a: boolean; - /** Immutable: Whether to use a source or mutable source under the hood */ - i: boolean; /** The associated proxy */ p: ProxyStateObject; /** The original target this proxy was created for */ From 3fd622190b6b915f35bee7506db21a33cb209165 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 8 Jul 2024 15:23:25 -0400 Subject: [PATCH 7/7] remove unused parameter --- .../src/compiler/phases/3-transform/client/utils.js | 1 - packages/svelte/src/internal/client/proxy.js | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 6137cfaecc17..e77ee236efeb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -454,7 +454,6 @@ export function serialize_proxy_reassignment(value, proxy_reference, state) { ? b.call( '$.proxy', value, - b.true, b.null, typeof proxy_reference === 'string' ? b.id(proxy_reference) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 2aebc35d6d97..f5900ba957c7 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -19,12 +19,11 @@ import * as e from './errors.js'; /** * @template T * @param {T} value - * @param {true} [immutable] * @param {import('#client').ProxyMetadata | null} [parent] * @param {import('#client').Source} [prev] dev mode only * @returns {import('#client').ProxyStateObject | T} */ -export function proxy(value, immutable = true, parent = null, prev) { +export function proxy(value, parent = null, prev) { if ( typeof value === 'object' && value != null && @@ -168,7 +167,7 @@ const state_proxy_handler = { const metadata = target[STATE_SYMBOL]; const s = metadata.s.get(prop); - if (s !== undefined) set(s, proxy(descriptor.value, true, metadata)); + if (s !== undefined) set(s, proxy(descriptor.value, metadata)); } return Reflect.defineProperty(target, prop, descriptor); @@ -214,7 +213,7 @@ const state_proxy_handler = { // create a source, but only if it's an own property and not a prototype property if (s === undefined && (!(prop in target) || get_descriptor(target, prop)?.writable)) { - s = source(proxy(target[prop], true, metadata)); + s = source(proxy(target[prop], metadata)); metadata.s.set(prop, s); } @@ -255,7 +254,7 @@ const state_proxy_handler = { (current_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = source(has ? proxy(target[prop], true, metadata) : UNINITIALIZED); + s = source(has ? proxy(target[prop], metadata) : UNINITIALIZED); metadata.s.set(prop, s); } const value = get(s); @@ -280,7 +279,7 @@ const state_proxy_handler = { s = metadata.s.get(prop); } if (s !== undefined) { - set(s, proxy(value, true, metadata)); + set(s, proxy(value, metadata)); } const is_array = metadata.a; const not_has = !(prop in target);