mirror of https://github.com/sveltejs/svelte
fix: rework binding ownership validation (#15678)
* remove old validation * fix: rework binding ownership validation Previously we were doing stack-based retrieval of the owner, which while catching more cases was costly (performance-wise) and prone to errors (as shown by many issues over the months). This drastically simplifies the ownership validation - we now only do simple static analysis to check which props are mutated and wrap them with runtime checks to see if a binding was established. Besides making the implementation simpler and more performant, this also follows an insight we had over the months: Most people don't really know what to do with this warning when it's shown beyond very simple cases. Either it's not actionable because they don't really know how to fix it or they question if they should at all (in some cases rightfully so). Now that the warning is only shown in simple and easy-to-reason-about cases, it has a much better signal-to-noise-ratio and will hopefully guide people in the right direction early on (learn from the obvious cases to not write spaghetti code in more complex cases). closes #15532 closes #15210 closes #14893 closes #13607 closes #13139 closes #11861 * remove some now obsolete tests * fix * better warnings now that we have more info * fix * hoist * we only care about mutation, not reassignment * tidy * handle prop aliases * mutation validation is only tangentially linked to context requirement * no need for two vars, one will do * update warning, include mutation location * tweak --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>pull/15705/head
Simon H
6 months ago
committed by
GitHub
parent
caf62ee687
commit
98d14ece66
@ -0,0 +1,5 @@
|
||||
---
|
||||
'svelte': patch
|
||||
---
|
||||
|
||||
fix: rework binding ownership validation
|
||||
@ -1,11 +0,0 @@
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
async test({ assert, warnings }) {
|
||||
assert.deepEqual(warnings, []);
|
||||
}
|
||||
});
|
||||
@ -1,18 +0,0 @@
|
||||
<script module>
|
||||
let toast1 = $state();
|
||||
let toast2 = $state({});
|
||||
|
||||
export async function show_toast() {
|
||||
toast1 = {
|
||||
message: 'foo',
|
||||
show: true
|
||||
};
|
||||
toast1.show = false;
|
||||
|
||||
toast2 = {
|
||||
message: 'foo',
|
||||
show: true
|
||||
};
|
||||
toast2.show = false;
|
||||
}
|
||||
</script>
|
||||
@ -1,5 +0,0 @@
|
||||
<script>
|
||||
import { show_toast } from "./child.svelte";
|
||||
|
||||
show_toast();
|
||||
</script>
|
||||
@ -0,0 +1,9 @@
|
||||
<script>
|
||||
import { setContext } from 'svelte';
|
||||
import Sub from './sub.svelte';
|
||||
|
||||
let list = $state([]);
|
||||
setContext('list', list);
|
||||
</script>
|
||||
|
||||
<Sub />
|
||||
@ -1,41 +1,24 @@
|
||||
import { flushSync } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
/** @type {typeof console.warn} */
|
||||
let warn;
|
||||
|
||||
/** @type {any[]} */
|
||||
let warnings = [];
|
||||
|
||||
export default test({
|
||||
html: `<button>clicks: 0</button>`,
|
||||
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
before_test: () => {
|
||||
warn = console.warn;
|
||||
|
||||
console.warn = (...args) => {
|
||||
warnings.push(...args);
|
||||
};
|
||||
},
|
||||
test({ assert, target, warnings }) {
|
||||
const [btn1, btn2] = target.querySelectorAll('button');
|
||||
|
||||
after_test: () => {
|
||||
console.warn = warn;
|
||||
warnings = [];
|
||||
},
|
||||
flushSync(() => {
|
||||
btn1.click();
|
||||
});
|
||||
|
||||
test({ assert, target }) {
|
||||
const btn = target.querySelector('button');
|
||||
assert.deepEqual(warnings.length, 0);
|
||||
|
||||
flushSync(() => {
|
||||
btn?.click();
|
||||
btn2.click();
|
||||
});
|
||||
|
||||
assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button>`);
|
||||
|
||||
assert.deepEqual(warnings, []);
|
||||
assert.deepEqual(warnings.length, 1);
|
||||
}
|
||||
});
|
||||
|
||||
@ -1,9 +1,8 @@
|
||||
<script>
|
||||
import { global } from './state.svelte.js';
|
||||
<script lang="ts">
|
||||
import Sub from './sub.svelte';
|
||||
import { create_my_state } from './state.svelte';
|
||||
|
||||
global.a = { b: 0 };
|
||||
const myState = create_my_state();
|
||||
</script>
|
||||
|
||||
<button onclick={() => global.a.b += 1}>
|
||||
clicks: {global.a.b}
|
||||
</button>
|
||||
<Sub count={myState.my_state} inc={myState.inc} />
|
||||
|
||||
@ -1 +1,14 @@
|
||||
export let global = $state({});
|
||||
export function create_my_state() {
|
||||
const my_state = $state({
|
||||
a: 0
|
||||
});
|
||||
|
||||
function inc() {
|
||||
my_state.a++;
|
||||
}
|
||||
|
||||
return {
|
||||
my_state,
|
||||
inc
|
||||
};
|
||||
}
|
||||
|
||||
@ -1,41 +1,24 @@
|
||||
import { flushSync } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
/** @type {typeof console.warn} */
|
||||
let warn;
|
||||
|
||||
/** @type {any[]} */
|
||||
let warnings = [];
|
||||
|
||||
export default test({
|
||||
html: `<button>[]</button>`,
|
||||
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
before_test: () => {
|
||||
warn = console.warn;
|
||||
|
||||
console.warn = (...args) => {
|
||||
warnings.push(...args);
|
||||
};
|
||||
},
|
||||
async test({ assert, target, warnings }) {
|
||||
const [btn1, btn2] = target.querySelectorAll('button');
|
||||
|
||||
after_test: () => {
|
||||
console.warn = warn;
|
||||
warnings = [];
|
||||
},
|
||||
flushSync(() => {
|
||||
btn1.click();
|
||||
});
|
||||
|
||||
test({ assert, target }) {
|
||||
const btn = target.querySelector('button');
|
||||
assert.deepEqual(warnings.length, 0);
|
||||
|
||||
flushSync(() => {
|
||||
btn?.click();
|
||||
btn2.click();
|
||||
});
|
||||
|
||||
assert.htmlEqual(target.innerHTML, `<button>[foo]</button>`);
|
||||
|
||||
assert.deepEqual(warnings, [], 'expected getContext to have widened ownership');
|
||||
assert.deepEqual(warnings.length, 1);
|
||||
}
|
||||
});
|
||||
|
||||
@ -1,9 +1,13 @@
|
||||
<script>
|
||||
import { setContext } from 'svelte';
|
||||
import Sub from './sub.svelte';
|
||||
import Child from './Child.svelte';
|
||||
|
||||
let list = $state([]);
|
||||
setContext('list', list);
|
||||
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>
|
||||
|
||||
<Sub />
|
||||
<button {onclick}>First click here</button>
|
||||
<Child item={found} />
|
||||
|
||||
@ -1,37 +0,0 @@
|
||||
import { flushSync } 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 = [];
|
||||
},
|
||||
|
||||
test({ assert, target }) {
|
||||
const btn = target.querySelector('button');
|
||||
|
||||
flushSync(() => {
|
||||
btn?.click();
|
||||
});
|
||||
|
||||
assert.deepEqual(warnings, []);
|
||||
}
|
||||
});
|
||||
@ -1,11 +0,0 @@
|
||||
<script>
|
||||
import { global } from './state.svelte.js';
|
||||
import Sub from './sub.svelte';
|
||||
</script>
|
||||
|
||||
<Sub />
|
||||
<!-- it's important _NOT_ to read global.a.b in the template,
|
||||
else the proxy would set up the structure with its own component context already -->
|
||||
<button onclick={() => global.increment_a_b()}>
|
||||
click me
|
||||
</button>
|
||||
@ -1,13 +0,0 @@
|
||||
class Global {
|
||||
state = $state({});
|
||||
|
||||
add_a(a) {
|
||||
this.state.a = a;
|
||||
}
|
||||
|
||||
increment_a_b() {
|
||||
this.state.a.b++;
|
||||
}
|
||||
}
|
||||
|
||||
export const global = new Global();
|
||||
@ -1,8 +0,0 @@
|
||||
<script>
|
||||
import { global } from "./state.svelte";
|
||||
|
||||
// The real world use case would be someone manipulating state locally, for example form state,
|
||||
// and then push the values to a global state for everyone else to see / possibly mutate.
|
||||
const local_soon_global = $state({ b: 0 });
|
||||
global.add_a(local_soon_global);
|
||||
</script>
|
||||
@ -1,24 +0,0 @@
|
||||
import { flushSync } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
test({ assert, target, warnings }) {
|
||||
const [btn1, btn2] = target.querySelectorAll('button');
|
||||
|
||||
flushSync(() => {
|
||||
btn1.click();
|
||||
});
|
||||
|
||||
assert.deepEqual(warnings.length, 0);
|
||||
|
||||
flushSync(() => {
|
||||
btn2.click();
|
||||
});
|
||||
|
||||
assert.deepEqual(warnings.length, 1);
|
||||
}
|
||||
});
|
||||
@ -1,8 +0,0 @@
|
||||
<script lang="ts">
|
||||
import Sub from './sub.svelte';
|
||||
import { create_my_state } from './state.svelte';
|
||||
|
||||
const myState = create_my_state();
|
||||
</script>
|
||||
|
||||
<Sub count={myState.my_state} inc={myState.inc} />
|
||||
@ -1,14 +0,0 @@
|
||||
export function create_my_state() {
|
||||
const my_state = $state({
|
||||
a: 0
|
||||
});
|
||||
|
||||
function inc() {
|
||||
my_state.a++;
|
||||
}
|
||||
|
||||
return {
|
||||
my_state,
|
||||
inc
|
||||
};
|
||||
}
|
||||
@ -1,9 +0,0 @@
|
||||
<script>
|
||||
import { getContext } from 'svelte';
|
||||
|
||||
const foo = getContext('foo')
|
||||
</script>
|
||||
|
||||
<button onclick={() => {
|
||||
foo.person.name.last = 'T';
|
||||
}}>mutate</button>
|
||||
@ -1,37 +0,0 @@
|
||||
import { flushSync } 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 btn = target.querySelector('button');
|
||||
|
||||
flushSync(() => {
|
||||
btn?.click();
|
||||
});
|
||||
|
||||
assert.deepEqual(warnings.length, 0);
|
||||
}
|
||||
});
|
||||
@ -1,17 +0,0 @@
|
||||
<script>
|
||||
import { setContext } from 'svelte';
|
||||
import Child from './Child.svelte';
|
||||
|
||||
class Person {
|
||||
name = $state({
|
||||
first: 'Mr',
|
||||
last: 'Bean'
|
||||
});
|
||||
}
|
||||
|
||||
setContext('foo', {
|
||||
person: new Person()
|
||||
});
|
||||
</script>
|
||||
|
||||
<Child />
|
||||
@ -1,24 +0,0 @@
|
||||
import { flushSync } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
async test({ assert, target, warnings }) {
|
||||
const [btn1, btn2] = target.querySelectorAll('button');
|
||||
|
||||
flushSync(() => {
|
||||
btn1.click();
|
||||
});
|
||||
|
||||
assert.deepEqual(warnings.length, 0);
|
||||
|
||||
flushSync(() => {
|
||||
btn2.click();
|
||||
});
|
||||
|
||||
assert.deepEqual(warnings.length, 1);
|
||||
}
|
||||
});
|
||||
@ -1,13 +0,0 @@
|
||||
<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} />
|
||||
@ -1,7 +0,0 @@
|
||||
<script>
|
||||
import { global } from './state.svelte.js';
|
||||
</script>
|
||||
|
||||
<button onclick={() => global.object.count += 1}>
|
||||
clicks: {global.object.count}
|
||||
</button>
|
||||
@ -1,9 +0,0 @@
|
||||
<script>
|
||||
import Counter from './Counter.svelte';
|
||||
import { global } from './state.svelte.js';
|
||||
|
||||
let object = $state({ count: 0 });
|
||||
global.object = object;
|
||||
</script>
|
||||
|
||||
<Counter />
|
||||
@ -1,3 +0,0 @@
|
||||
export let global = $state({
|
||||
object: { count: -1 }
|
||||
});
|
||||
@ -1,7 +0,0 @@
|
||||
<script>
|
||||
let { linked3 = $bindable(), linked4 = $bindable() } = $props();
|
||||
</script>
|
||||
|
||||
<p>Binding</p>
|
||||
<button onclick={() => linked3.count++}>Increment Linked 1 ({linked3.count})</button>
|
||||
<button onclick={() => linked4.count++}>Increment Linked 2 ({linked4.count})</button>
|
||||
@ -1,13 +0,0 @@
|
||||
<script>
|
||||
import { getContext } from 'svelte';
|
||||
const linked1 = getContext('linked1');
|
||||
const linked2 = getContext('linked2');
|
||||
</script>
|
||||
|
||||
<p>Context</p>
|
||||
<button onclick={() => linked1.linked.current.count++}
|
||||
>Increment Linked 1 ({linked1.linked.current.count})</button
|
||||
>
|
||||
<button onclick={() => linked2.linked.current.count++}
|
||||
>Increment Linked 2 ({linked2.linked.current.count})</button
|
||||
>
|
||||
@ -1,34 +0,0 @@
|
||||
import { flushSync } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
// Tests that ownership is widened with $derived (on class or on its own) that contains $state
|
||||
export default test({
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
test({ assert, target, warnings }) {
|
||||
const [root, counter_context1, counter_context2, counter_binding1, counter_binding2] =
|
||||
target.querySelectorAll('button');
|
||||
|
||||
counter_context1.click();
|
||||
counter_context2.click();
|
||||
counter_binding1.click();
|
||||
counter_binding2.click();
|
||||
flushSync();
|
||||
|
||||
assert.equal(warnings.length, 0);
|
||||
|
||||
root.click();
|
||||
flushSync();
|
||||
counter_context1.click();
|
||||
counter_context2.click();
|
||||
counter_binding1.click();
|
||||
counter_binding2.click();
|
||||
flushSync();
|
||||
|
||||
assert.equal(warnings.length, 0);
|
||||
},
|
||||
|
||||
warnings: []
|
||||
});
|
||||
@ -1,46 +0,0 @@
|
||||
<script>
|
||||
import CounterBinding from './CounterBinding.svelte';
|
||||
import CounterContext from './CounterContext.svelte';
|
||||
import { setContext } from 'svelte';
|
||||
|
||||
let counter = $state({ count: 0 });
|
||||
|
||||
class Linked {
|
||||
#getter;
|
||||
linked = $derived.by(() => {
|
||||
const state = $state({ current: $state.snapshot(this.#getter()) });
|
||||
return state;
|
||||
});
|
||||
|
||||
constructor(fn) {
|
||||
this.#getter = fn;
|
||||
}
|
||||
}
|
||||
|
||||
const linked1 = $derived.by(() => {
|
||||
const state = $state({ current: $state.snapshot(counter) });
|
||||
return state;
|
||||
});
|
||||
const linked2 = new Linked(() => counter);
|
||||
|
||||
setContext('linked1', {
|
||||
get linked() {
|
||||
return linked1;
|
||||
}
|
||||
});
|
||||
setContext('linked2', linked2);
|
||||
|
||||
const linked3 = $derived.by(() => {
|
||||
const state = $state({ current: $state.snapshot(counter) });
|
||||
return state;
|
||||
});
|
||||
const linked4 = new Linked(() => counter);
|
||||
</script>
|
||||
|
||||
<p>Parent</p>
|
||||
<button onclick={() => counter.count++}>
|
||||
Increment Original ({counter.count})
|
||||
</button>
|
||||
|
||||
<CounterContext />
|
||||
<CounterBinding bind:linked3={linked3.current} bind:linked4={linked4.linked.current} />
|
||||
Loading…
Reference in new issue