Skip to content

Commit f93a6ce

Browse files
committed
fix: check effect.pre in assign-in-effect
It seems like the intention here was to check `$effect` _and_ `$effect.pre`, but the condition (`in_effect`) was filtering those out. The `is_rune` helper already does the same checks, so we can just remove the ones here and delegate to that.
1 parent cb316c5 commit f93a6ce

File tree

2 files changed

+23
-15
lines changed

2 files changed

+23
-15
lines changed

packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe('add_autofixers_issues', () => {
6161
<script>
6262
const count = ${init}(0);
6363
</script>
64-
64+
6565
<button onclick={() => count = 43}>Increment</button>
6666
`);
6767

@@ -74,7 +74,7 @@ describe('add_autofixers_issues', () => {
7474
const content = run_autofixers_on_code(`
7575
<script>
7676
const count = 0;
77-
77+
7878
$effect(() => {
7979
count = 43;
8080
});
@@ -89,7 +89,7 @@ describe('add_autofixers_issues', () => {
8989
const content = run_autofixers_on_code(`
9090
<script>
9191
let count = ${init}(0);
92-
92+
9393
$effect(() => {
9494
count++;
9595
});
@@ -105,7 +105,7 @@ describe('add_autofixers_issues', () => {
105105
const content = run_autofixers_on_code(`
106106
<script>
107107
let count = ${init}({ value: 0 });
108-
108+
109109
$effect(() => {
110110
count.value = 42;
111111
});
@@ -116,6 +116,22 @@ describe('add_autofixers_issues', () => {
116116
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
117117
);
118118
});
119+
120+
it(`should add a suggestion for variables that are mutated within an effect.pre`, () => {
121+
const content = run_autofixers_on_code(`
122+
<script>
123+
let count = ${init}({ value: 0 });
124+
125+
$effect.pre(() => {
126+
count.value = 42;
127+
});
128+
</script>
129+
`);
130+
131+
expect(content.suggestions).toContain(
132+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
133+
);
134+
});
119135
});
120136
});
121137

packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,11 @@ function run_if_in_effect(
1111
) {
1212
const in_effect = path.findLast(
1313
(node) =>
14-
node.type === 'CallExpression' &&
15-
node.callee.type === 'Identifier' &&
16-
node.callee.name === '$effect',
14+
node.type === 'CallExpression' && state.parsed.is_rune(node, ['$effect', '$effect.pre']),
1715
);
1816

19-
if (
20-
in_effect &&
21-
in_effect.type === 'CallExpression' &&
22-
(in_effect.callee.type === 'Identifier' || in_effect.callee.type === 'MemberExpression')
23-
) {
24-
if (state.parsed.is_rune(in_effect, ['$effect', '$effect.pre'])) {
25-
to_run();
26-
}
17+
if (in_effect) {
18+
to_run();
2719
}
2820
}
2921

0 commit comments

Comments
 (0)