fix: more conservative assignment_value_stale warnings (#17574)

main
Rich Harris 5 hours ago committed by GitHub
parent 2b4253f332
commit 37cd40d2e5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: more conservative assignment_value_stale warnings

@ -162,7 +162,10 @@ function build_assignment(operator, left, right, context) {
// will be pushed to. we do this by transforming it to something like
// `$.assign_nullish(object, 'items', [])`
let should_transform =
dev && path.at(-1) !== 'ExpressionStatement' && is_non_coercive_operator(operator);
dev &&
path.at(-1) !== 'ExpressionStatement' &&
is_non_coercive_operator(operator) &&
!context.state.scope.evaluate(right).is_primitive;
// special case — ignore `onclick={() => (...)}`
if (

@ -228,6 +228,13 @@ class Evaluation {
*/
is_number = true;
/**
* True if the value is known to be a primitive
* @readonly
* @type {boolean}
*/
is_primitive = true;
/**
* True if the value is known to be a function
* @readonly
@ -577,6 +584,7 @@ class Evaluation {
if (value === UNKNOWN) {
this.has_unknown = true;
this.is_primitive = false;
}
}

@ -1,3 +1,4 @@
import { STATE_SYMBOL } from '#client/constants';
import { sanitize_location } from '../../../utils.js';
import { untrack } from '../runtime.js';
import * as w from '../warnings.js';
@ -10,7 +11,7 @@ import * as w from '../warnings.js';
* @param {string} location
*/
function compare(a, b, property, location) {
if (a !== b) {
if (a !== b && typeof b === 'object' && STATE_SYMBOL in b) {
w.assignment_value_stale(property, /** @type {string} */ (sanitize_location(location)));
}

@ -6,31 +6,26 @@ export default test({
dev: true
},
html: `<button>items: null</button> <div>x</div> <input type="checkbox" value="1"><input type="checkbox" value="2"><input>`,
test({ assert, target, warnings }) {
const btn = target.querySelector('button');
ok(btn);
const [button1, button2, button3] = target.querySelectorAll('button');
ok(button1);
flushSync(() => btn.click());
assert.htmlEqual(
target.innerHTML,
`<button>items: []</button> <div>x</div> <input type="checkbox" value="1"><input type="checkbox" value="2"><input>`
);
flushSync(() => button1.click());
assert.htmlEqual(button1.innerHTML, `items: []`);
flushSync(() => btn.click());
assert.htmlEqual(
target.innerHTML,
`<button>items: [0]</button> <div>x</div> <input type="checkbox" value="1"><input type="checkbox" value="2"><input>`
);
flushSync(() => button1.click());
assert.htmlEqual(button1.innerHTML, `items: [0]`);
const input = target.querySelector('input');
ok(input);
input.checked = true;
flushSync(() => input.dispatchEvent(new Event('change', { bubbles: true })));
flushSync(() => button2.click());
flushSync(() => button3.click());
assert.deepEqual(warnings, [
'Assignment to `items` property (main.svelte:9:24) will evaluate to the right-hand side, not the value of `items` following the assignment. This may result in unexpected behaviour.'
'Assignment to `items` property (main.svelte:17:24) will evaluate to the right-hand side, not the value of `items` following the assignment. This may result in unexpected behaviour.'
]);
}
});

@ -1,9 +1,17 @@
<script>
import Test from './Test.svelte';
let { opacity = 0.5 } = $props();
let entries = $state([]);
let object = $state({ items: null, group: [] });
let elementFunBind = $state();
// should omit $.assign via static analysis
const fixed = (node) => node.style.opacity = 0.5;
// should use $.assign, but it should not warn
const unknown = (node) => node.style.opacity = opacity;
</script>
<button onclick={() => (object.items ??= []).push(object.items.length)}>
@ -23,3 +31,6 @@
<input bind:this={() => {}, (e) => (context.element = e)} />
{/snippet}
{@render funBind({ set element(e) { elementFunBind = e } })}
<button onclick={(e) => (fixed(e.currentTarget))}>change opacity (fixed)</button>
<button onclick={(e) => (unknown(e.currentTarget))}>change opacity (unknown)</button>

Loading…
Cancel
Save