Skip to content

Commit

Permalink
fix: don't emit assignment warnings for bindings (#14651)
Browse files Browse the repository at this point in the history
Also fixes the possibility of an infinite loop due to the property access during the dev time check

fixes #14643
  • Loading branch information
dummdidumm authored Dec 10, 2024
1 parent 66e30d3 commit 88184cd
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 16 deletions.
5 changes: 5 additions & 0 deletions .changeset/thick-dryers-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: don't emit assignment warnings for bindings
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,17 @@ function build_assignment(operator, left, right, context) {
}
}

// special case — ignore `bind:prop={getter, (v) => (...)}` / `bind:value={x.y}`
if (
path.at(-1) === 'Component' ||
path.at(-1) === 'SvelteComponent' ||
(path.at(-1) === 'ArrowFunctionExpression' &&
path.at(-2) === 'SequenceExpression' &&
(path.at(-3) === 'Component' || path.at(-3) === 'SvelteComponent'))
) {
should_transform = false;
}

if (left.type === 'MemberExpression' && should_transform) {
const callee = callees[operator];

Expand Down
29 changes: 25 additions & 4 deletions packages/svelte/src/internal/client/dev/assign.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { untrack } from '../runtime.js';
import * as w from '../warnings.js';
import { sanitize_location } from './location.js';

Expand All @@ -23,7 +24,12 @@ function compare(a, b, property, location) {
* @param {string} location
*/
export function assign(object, property, value, location) {
return compare((object[property] = value), object[property], property, location);
return compare(
(object[property] = value),
untrack(() => object[property]),
property,
location
);
}

/**
Expand All @@ -33,7 +39,12 @@ export function assign(object, property, value, location) {
* @param {string} location
*/
export function assign_and(object, property, value, location) {
return compare((object[property] &&= value), object[property], property, location);
return compare(
(object[property] &&= value),
untrack(() => object[property]),
property,
location
);
}

/**
Expand All @@ -43,7 +54,12 @@ export function assign_and(object, property, value, location) {
* @param {string} location
*/
export function assign_or(object, property, value, location) {
return compare((object[property] ||= value), object[property], property, location);
return compare(
(object[property] ||= value),
untrack(() => object[property]),
property,
location
);
}

/**
Expand All @@ -53,5 +69,10 @@ export function assign_or(object, property, value, location) {
* @param {string} location
*/
export function assign_nullish(object, property, value, location) {
return compare((object[property] ??= value), object[property], property, location);
return compare(
(object[property] ??= value),
untrack(() => object[property]),
property,
location
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
let { x = $bindable() } = $props();
$effect(() => {
x = {};
});
export function soThatTestReturnsAnObject() {
return x;
}
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ export default test({
dev: true
},

html: `<button>items: null</button>`,
html: `<button>items: null</button> <div>x</div>`,

test({ assert, target, warnings }) {
const btn = target.querySelector('button');

flushSync(() => btn?.click());
assert.htmlEqual(target.innerHTML, `<button>items: []</button>`);
assert.htmlEqual(target.innerHTML, `<button>items: []</button> <div>x</div>`);

flushSync(() => btn?.click());
assert.htmlEqual(target.innerHTML, `<button>items: [0]</button>`);
assert.htmlEqual(target.innerHTML, `<button>items: [0]</button> <div>x</div>`);

assert.deepEqual(warnings, [
'Assignment to `items` property (main.svelte:5: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:8:24) will evaluate to the right-hand side, not the value of `items` following the assignment. This may result in unexpected behaviour.'
]);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script>
import Test from './Test.svelte';
let entries = $state([]);
let object = $state({ items: null });
</script>

<button onclick={() => (object.items ??= []).push(object.items.length)}>
items: {JSON.stringify(object.items)}
</button>

<!-- these should not emit warnings -->
<div bind:this={entries[0]}>x</div>
<Test bind:this={entries[1]}></Test>
<Test bind:this={() => entries[2], (v) => (entries[2] = v)}></Test>
<Test bind:x={entries[3]}></Test>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default test({
html: `<button>items: null</button>`,

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

flushSync(() => btn1.click());
assert.htmlEqual(target.innerHTML, `<button>items: [0]</button>`);
Expand Down

This file was deleted.

0 comments on commit 88184cd

Please sign in to comment.