Skip to content

Commit

Permalink
fix: widen ownership when sub state is assigned to new state (#11217)
Browse files Browse the repository at this point in the history
Ownership was not widened when assigning a sub state to a different top level state. The set of owners for the state was zero because the owner was on the original parent, but that one was reset to null because it's now the top level of a different state. That meant that there was no owner but also no parent to check for the owner, which is an invalid combination resulting in a nullpointer (and also potentially false positive warnings in other situations).

fixes #11204
  • Loading branch information
dummdidumm authored Apr 18, 2024
1 parent c44234d commit 4b59ef3
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/weak-frogs-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: widen ownership when sub state is assigned to new state
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/dev/ownership.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ export function add_owner(object, owner, global = false) {
}

/**
* @param {import('#client').ProxyMetadata | null} from
* @param {import('#client').ProxyMetadata} to
* @param {import('#client').ProxyMetadata<any> | null} from
* @param {import('#client').ProxyMetadata<any>} to
*/
export function widen_ownership(from, to) {
if (to.owners === null) {
Expand Down
3 changes: 3 additions & 0 deletions packages/svelte/src/internal/client/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export function proxy(value, immutable = true, parent = null) {
// someone copied the state symbol using `Reflect.ownKeys(...)`
if (metadata.t === value || metadata.p === value) {
if (DEV) {
// Since original parent relationship gets lost, we need to copy over ancestor owners
// into current metadata. The object might still exist on both, so we need to widen it.
widen_ownership(metadata, metadata);
metadata.parent = parent;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
let { item } = $props();
function onclick() {
item.name = `${item.name} edited`
}
</script>

<div>{item?.name}</div>
<button {onclick}>Then click here</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { tick } from 'svelte';
import { test } from '../../test';

/** @type {typeof console.warn} */
let warn;

/** @type {any[]} */
let warnings = [];

export default test({
compileOptions: {
dev: true
},

before_test: () => {
warn = console.warn;

console.warn = (...args) => {
warnings.push(...args);
};
},

after_test: () => {
console.warn = warn;
warnings = [];
},

async test({ assert, target }) {
const [btn1, btn2] = target.querySelectorAll('button');

btn1.click();
await tick();

assert.deepEqual(warnings.length, 0);

btn2.click();
await tick();

assert.deepEqual(warnings.length, 1);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
import Child from './Child.svelte';
let items = $state([{ id: "test", name: "this is a test"}, { id:"test2", name: "this is a second test"}]);
let found = $state();
function onclick() {
found = items.find(c => c.id === 'test2');
}
</script>

<button {onclick}>First click here</button>
<Child item={found} />

0 comments on commit 4b59ef3

Please sign in to comment.