Skip to content

Commit

Permalink
fix: don't emit assignment warnings for bindings
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 committed Dec 10, 2024
1 parent 8bee23e commit cc9c43e
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 cc9c43e

Please sign in to comment.