Skip to content

fix: resolve legacy component props equality for mutations #12348

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 8 commits into from
Jul 8, 2024
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/six-chicken-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: resolve legacy component props equality for mutations
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 6 additions & 10 deletions packages/svelte/src/internal/client/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,19 @@ 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';

/**
* @template T
* @param {T} value
* @param {boolean} [immutable]
* @param {import('#client').ProxyMetadata | null} [parent]
* @param {import('#client').Source<T>} [prev] dev mode only
* @returns {import('#client').ProxyStateObject<T> | T}
*/
export function proxy(value, immutable = true, parent = null, prev) {
export function proxy(value, parent = null, prev) {
if (
typeof value === 'object' &&
value != null &&
Expand Down Expand Up @@ -59,7 +58,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
}),
Expand Down Expand Up @@ -169,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, metadata.i, metadata));
if (s !== undefined) set(s, proxy(descriptor.value, metadata));
}

return Reflect.defineProperty(target, prop, descriptor);
Expand Down Expand Up @@ -215,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 = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata));
s = source(proxy(target[prop], metadata));
metadata.s.set(prop, s);
}

Expand Down Expand Up @@ -256,9 +254,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], metadata) : UNINITIALIZED);
metadata.s.set(prop, s);
}
const value = get(s);
Expand All @@ -283,7 +279,7 @@ const state_proxy_handler = {
s = metadata.s.get(prop);
}
if (s !== undefined) {
set(s, proxy(value, metadata.i, metadata));
set(s, proxy(value, metadata));
}
const is_array = metadata.a;
const not_has = !(prop in target);
Expand Down
2 changes: 0 additions & 2 deletions packages/svelte/src/internal/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ export interface ProxyMetadata<T = Record<string | symbol, any>> {
v: Source<number>;
/** `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<T>;
/** The original target this proxy was created for */
Expand Down
52 changes: 47 additions & 5 deletions packages/svelte/src/legacy/legacy-client.js
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -69,10 +70,51 @@ class Svelte4Component {
* }} options
*/
constructor(options) {
// 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);
var sources = new Map();
var add_source = (/** @type {string | symbol} */ key) => {
var s = mutable_source(0);
sources.set(key, s);
return s;
};
// 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: {} },
{
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,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { test } from '../../test';

const data = {
message: 'hello'
};

export default test({
get props() {
data.message = 'hello';

return {
data
};
},

html: '<p>hello</p>',

async test({ assert, component, target }) {
data.message = 'goodbye';
await component.$set({ data });

assert.htmlEqual(target.innerHTML, '<p>goodbye</p>');
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
export let data;
</script>

<p>{data.message}</p>
Loading