Skip to content

Commit 8a808ab

Browse files
committed
fix more cases
1 parent 9883af4 commit 8a808ab

File tree

6 files changed

+58
-40
lines changed

6 files changed

+58
-40
lines changed

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export interface ClientTransformState extends TransformState {
3131
/** turn `foo = bar` into e.g. `$.set(foo, bar)` */
3232
assign?: (node: Identifier, value: Expression) => Expression;
3333
/** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */
34-
mutate?: (node: Identifier, mutation: AssignmentExpression) => Expression;
34+
mutate?: (node: Identifier, mutation: AssignmentExpression | UpdateExpression) => Expression;
3535
/** turn `foo++` into e.g. `$.update(foo)` */
3636
update?: (node: UpdateExpression) => Expression;
3737
}

packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,10 @@ export function EachBlock(node, context) {
214214

215215
return b.sequence([b.assignment('=', left, value), ...sequence]);
216216
},
217-
mutate: (_, mutation) => b.sequence([mutation, ...sequence])
217+
mutate: (_, mutation) => {
218+
uses_index = true;
219+
return b.sequence([mutation, ...sequence]);
220+
}
218221
};
219222

220223
delete key_state.transform[node.context.name];

packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,15 @@ export function Program(_, context) {
7777
return b.call(
7878
'$.store_mutate',
7979
get_store(),
80-
b.assignment(
81-
mutation.operator,
82-
/** @type {MemberExpression} */ (
83-
replace(/** @type {MemberExpression} */ (mutation.left))
84-
),
85-
mutation.right
86-
),
80+
mutation.type === 'AssignmentExpression'
81+
? b.assignment(
82+
mutation.operator,
83+
/** @type {MemberExpression} */ (
84+
replace(/** @type {MemberExpression} */ (mutation.left))
85+
),
86+
mutation.right
87+
)
88+
: b.update(mutation.operator, replace(mutation.argument), mutation.prefix),
8789
untracked
8890
);
8991
},
Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** @import { Expression, Node, Pattern, Statement, UpdateExpression } from 'estree' */
1+
/** @import { AssignmentExpression, Expression, UpdateExpression } from 'estree' */
22
/** @import { Context } from '../types' */
33
import { is_ignored } from '../../../../state.js';
44
import { object } from '../../../../utils/ast.js';
@@ -34,42 +34,24 @@ export function UpdateExpression(node, context) {
3434
}
3535

3636
const left = object(argument);
37+
const transformers = left && context.state.transform[left.name];
3738

38-
if (left === argument) {
39-
const transform = context.state.transform;
40-
const update = transform[left.name]?.update;
41-
42-
if (update && Object.hasOwn(transform, left.name)) {
43-
return update(node);
44-
}
39+
if (left === argument && transformers?.update) {
40+
// we don't need to worry about ownership_invalid_mutation here, because
41+
// we're not mutating but reassigning
42+
return transformers.update(node);
4543
}
4644

47-
/** @type {Expression} */
48-
let result;
45+
let update = /** @type {Expression} */ (context.next());
4946

50-
if (left === null || !context.state.transform[left.name]) {
51-
result = /** @type {Expression} */ (context.next());
52-
} else {
53-
const assignment = /** @type {Expression} */ (
54-
context.visit(
55-
b.assignment(
56-
node.operator === '++' ? '+=' : '-=',
57-
/** @type {Pattern} */ (argument),
58-
b.literal(1)
59-
)
60-
)
47+
if (left && transformers?.mutate) {
48+
update = transformers.mutate(
49+
left,
50+
/** @type {AssignmentExpression | UpdateExpression} */ (update)
6151
);
62-
63-
const parent = /** @type {Node} */ (context.path.at(-1));
64-
const is_standalone = parent.type === 'ExpressionStatement'; // TODO and possibly others, but not e.g. the `test` of a WhileStatement
65-
66-
result =
67-
node.prefix || is_standalone
68-
? assignment
69-
: b.binary(node.operator === '++' ? '-' : '+', assignment, b.literal(1));
7052
}
7153

7254
return is_ignored(node, 'ownership_invalid_mutation')
73-
? b.call('$.skip_ownership_validation', b.thunk(result))
74-
: result;
55+
? b.call('$.skip_ownership_validation', b.thunk(update))
56+
: update;
7557
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
html: '<button>mutate</button><button>reassign</button><p>0</p>',
6+
test({ assert, target }) {
7+
const [btn1, btn2] = target.querySelectorAll('button');
8+
9+
flushSync(() => btn1.click());
10+
assert.htmlEqual(target.innerHTML, '<button>mutate</button><button>reassign</button><p>1</p>');
11+
12+
flushSync(() => btn2.click());
13+
assert.htmlEqual(target.innerHTML, '<button>mutate</button><button>reassign</button><p>0</p>');
14+
}
15+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<script>
2+
let object = $state({ n: 0n });
3+
4+
function reassign() {
5+
object = { n: 0n };
6+
}
7+
8+
function mutate() {
9+
return object.n++;
10+
}
11+
</script>
12+
13+
<button onclick={mutate}>mutate</button>
14+
<button onclick={reassign}>reassign</button>
15+
16+
<p>{object.n}</p>

0 commit comments

Comments
 (0)