Skip to content

Commit 4c6232a

Browse files
paoloricciutipaoloricciuti
committed
fix: add $derived and $derived.by to the reassignment list
1 parent 8edbf2f commit 4c6232a

File tree

2 files changed

+91
-84
lines changed

2 files changed

+91
-84
lines changed

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

Lines changed: 90 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -12,99 +12,106 @@ function run_autofixers_on_code(code: string, desired_svelte_version = 5) {
1212

1313
describe('add_autofixers_issues', () => {
1414
describe('assign_in_effect', () => {
15-
it(`should add suggestions when assigning to a stateful variable inside an effect`, () => {
16-
const content = run_autofixers_on_code(`
17-
<script>
18-
const count = $state(0);
19-
$effect(() => {
20-
count = 43;
21-
});
22-
</script>`);
15+
describe.each([
16+
{ init: '$state' },
17+
{ init: '$state.raw' },
18+
{ init: '$derived' },
19+
{ init: '$derived.by' },
20+
])('($init)', ({ init }) => {
21+
it(`should add suggestions when assigning to a stateful variable inside an effect`, () => {
22+
const content = run_autofixers_on_code(`
23+
<script>
24+
const count = ${init}(0);
25+
$effect(() => {
26+
count = 43;
27+
});
28+
</script>`);
2329

24-
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
25-
expect(content.suggestions).toContain(
26-
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
27-
);
28-
});
30+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
31+
expect(content.suggestions).toContain(
32+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
33+
);
34+
});
2935

30-
it(`should add a suggestion for each variable assigned within an effect`, () => {
31-
const content = run_autofixers_on_code(`
32-
<script>
33-
const count = $state(0);
34-
const count2 = $state(0);
35-
$effect(() => {
36-
count = 43;
37-
count2 = 44;
38-
});
39-
</script>`);
36+
it(`should add a suggestion for each variable assigned within an effect`, () => {
37+
const content = run_autofixers_on_code(`
38+
<script>
39+
const count = $state(0);
40+
const count2 = $state(0);
41+
$effect(() => {
42+
count = 43;
43+
count2 = 44;
44+
});
45+
</script>`);
4046

41-
expect(content.suggestions.length).toBeGreaterThanOrEqual(2);
42-
expect(content.suggestions).toContain(
43-
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
44-
);
45-
expect(content.suggestions).toContain(
46-
'The stateful variable "count2" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
47-
);
48-
});
49-
it(`should not add a suggestion for variables that are not assigned within an effect`, () => {
50-
const content = run_autofixers_on_code(`
51-
<script>
52-
const count = $state(0);
53-
</script>
54-
55-
<button onclick={() => count = 43}>Increment</button>
56-
`);
47+
expect(content.suggestions.length).toBeGreaterThanOrEqual(2);
48+
expect(content.suggestions).toContain(
49+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
50+
);
51+
expect(content.suggestions).toContain(
52+
'The stateful variable "count2" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
53+
);
54+
});
55+
it(`should not add a suggestion for variables that are not assigned within an effect`, () => {
56+
const content = run_autofixers_on_code(`
57+
<script>
58+
const count = ${init}(0);
59+
</script>
60+
61+
<button onclick={() => count = 43}>Increment</button>
62+
`);
5763

58-
expect(content.suggestions).not.toContain(
59-
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
60-
);
61-
});
64+
expect(content.suggestions).not.toContain(
65+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
66+
);
67+
});
6268

63-
it("should not add a suggestions for variables that are assigned within an effect but aren't stateful", () => {
64-
const content = run_autofixers_on_code(`
65-
<script>
66-
const count = 0;
67-
68-
$effect(() => {
69-
count = 43;
70-
});
71-
</script>`);
69+
it("should not add a suggestions for variables that are assigned within an effect but aren't stateful", () => {
70+
const content = run_autofixers_on_code(`
71+
<script>
72+
const count = 0;
73+
74+
$effect(() => {
75+
count = 43;
76+
});
77+
</script>`);
7278

73-
expect(content.suggestions).not.toContain(
74-
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
75-
);
76-
});
79+
expect(content.suggestions).not.toContain(
80+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
81+
);
82+
});
7783

78-
it(`should add a suggestion for variables that are assigned within an effect with an update`, () => {
79-
const content = run_autofixers_on_code(`
80-
<script>
81-
let count = $state(0);
82-
83-
$effect(() => {
84-
count++;
85-
});
86-
</script>
87-
`);
84+
it(`should add a suggestion for variables that are assigned within an effect with an update`, () => {
85+
const content = run_autofixers_on_code(`
86+
<script>
87+
let count = ${init}(0);
88+
89+
$effect(() => {
90+
count++;
91+
});
92+
</script>
93+
`);
8894

89-
expect(content.suggestions).toContain(
90-
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
91-
);
92-
});
95+
expect(content.suggestions).toContain(
96+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
97+
);
98+
});
9399

94-
it(`should add a suggestion for variables that are mutated within an effect`, () => {
95-
const content = run_autofixers_on_code(`
96-
<script>
97-
let count = $state({ value: 0 });
98-
99-
$effect(() => {
100-
count.value = 42;
101-
});
102-
</script>
103-
`);
100+
it(`should add a suggestion for variables that are mutated within an effect`, () => {
101+
const content = run_autofixers_on_code(`
102+
<script>
103+
let count = ${init}({ value: 0 });
104+
105+
$effect(() => {
106+
count.value = 42;
107+
});
108+
</script>
109+
`);
104110

105-
expect(content.suggestions).toContain(
106-
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
107-
);
111+
expect(content.suggestions).toContain(
112+
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
113+
);
114+
});
108115
});
109116
});
110117

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function visitor(
3939
const init = definition.node.init;
4040
if (
4141
init?.type === 'CallExpression' &&
42-
state.parsed.is_rune(init, ['$state', '$state.raw'])
42+
state.parsed.is_rune(init, ['$state', '$state.raw', '$derived', '$derived.by'])
4343
) {
4444
state.output.suggestions.push(
4545
`The stateful variable "${id.name}" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.`,

0 commit comments

Comments
 (0)