Skip to content

Commit d4edef1

Browse files
paoloricciutipaoloricciuti
authored
Merge pull request #8 from sveltejs/set-update-autofixer
feat: add `set` or `update` stateful variables autofixer & imported runes autofixer
2 parents 13c4832 + 13719d0 commit d4edef1

File tree

8 files changed

+398
-50
lines changed

8 files changed

+398
-50
lines changed

‎src/lib/constants.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
export const base_runes = [
2+
'$state',
3+
'$effect',
4+
'$derived',
5+
'$inspect',
6+
'$props',
7+
'$bindable',
8+
'$host',
9+
] as const;
10+
11+
export const nested_runes = [
12+
'$state.raw',
13+
'$state.snapshot',
14+
'$effect.pre',
15+
'$effect.tracking',
16+
'$effect.pending',
17+
'$effect.root',
18+
'$derived.by',
19+
'$inspect.trace',
20+
'$props.id',
21+
] as const;
22+
23+
export const runes = [...base_runes, ...nested_runes] as const;

‎src/lib/mcp/autofixers/add-autofixers-issues.test.ts‎

Lines changed: 264 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,42 @@
11
import { describe, expect, it } from 'vitest';
2-
import { add_autofixers_issues } from './add-autofixers-issues';
2+
import { add_autofixers_issues } from './add-autofixers-issues.js';
3+
import { base_runes } from '../../constants.js';
4+
5+
const dollarless_runes = base_runes.map((r) => ({ rune: r.replace('$', '') }));
6+
7+
function run_autofixers_on_code(code: string, desired_svelte_version = 5) {
8+
const content = { issues: [], suggestions: [] };
9+
add_autofixers_issues(content, code, desired_svelte_version);
10+
return content;
11+
}
312

413
describe('add_autofixers_issues', () => {
514
describe('assign_in_effect', () => {
6-
it('should add suggestions when assigning to a stateful variable inside an effect', () => {
7-
const content = { issues: [], suggestions: [] };
8-
const code = `
15+
it(`should add suggestions when assigning to a stateful variable inside an effect`, () => {
16+
const content = run_autofixers_on_code(`
917
<script>
1018
const count = $state(0);
1119
$effect(() => {
1220
count = 43;
1321
});
14-
</script>`;
15-
add_autofixers_issues(content, code, 5);
22+
</script>`);
1623

1724
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
1825
expect(content.suggestions).toContain(
1926
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
2027
);
2128
});
2229

23-
it('should add a suggestion for each variable assigned within an effect', () => {
24-
const content = { issues: [], suggestions: [] };
25-
const code = `
30+
it(`should add a suggestion for each variable assigned within an effect`, () => {
31+
const content = run_autofixers_on_code(`
2632
<script>
2733
const count = $state(0);
2834
const count2 = $state(0);
2935
$effect(() => {
3036
count = 43;
3137
count2 = 44;
3238
});
33-
</script>`;
34-
add_autofixers_issues(content, code, 5);
39+
</script>`);
3540

3641
expect(content.suggestions.length).toBeGreaterThanOrEqual(2);
3742
expect(content.suggestions).toContain(
@@ -41,73 +46,302 @@ describe('add_autofixers_issues', () => {
4146
'The stateful variable "count2" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
4247
);
4348
});
44-
it('should not add a suggestion for variables that are not assigned within an effect', () => {
45-
const content = { issues: [], suggestions: [] };
46-
const code = `
49+
it(`should not add a suggestion for variables that are not assigned within an effect`, () => {
50+
const content = run_autofixers_on_code(`
4751
<script>
4852
const count = $state(0);
4953
</script>
5054
5155
<button onclick={() => count = 43}>Increment</button>
52-
`;
53-
add_autofixers_issues(content, code, 5);
56+
`);
5457

5558
expect(content.suggestions).not.toContain(
5659
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
5760
);
5861
});
5962

6063
it("should not add a suggestions for variables that are assigned within an effect but aren't stateful", () => {
61-
const content = { issues: [], suggestions: [] };
62-
const code = `
64+
const content = run_autofixers_on_code(`
6365
<script>
6466
const count = 0;
6567
6668
$effect(() => {
6769
count = 43;
6870
});
69-
</script>`;
70-
add_autofixers_issues(content, code, 5);
71+
</script>`);
7172

7273
expect(content.suggestions).not.toContain(
7374
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
7475
);
7576
});
7677

77-
it('should add a suggestion for variables that are assigned within an effect with an update', () => {
78-
const content = { issues: [], suggestions: [] };
79-
const code = `
78+
it(`should add a suggestion for variables that are assigned within an effect with an update`, () => {
79+
const content = run_autofixers_on_code(`
8080
<script>
8181
let count = $state(0);
8282
8383
$effect(() => {
8484
count++;
8585
});
8686
</script>
87-
`;
88-
add_autofixers_issues(content, code, 5);
87+
`);
8988

9089
expect(content.suggestions).toContain(
9190
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
9291
);
9392
});
9493

95-
it('should add a suggestion for variables that are mutated within an effect', () => {
96-
const content = { issues: [], suggestions: [] };
97-
const code = `
94+
it(`should add a suggestion for variables that are mutated within an effect`, () => {
95+
const content = run_autofixers_on_code(`
9896
<script>
9997
let count = $state({ value: 0 });
10098
10199
$effect(() => {
102100
count.value = 42;
103101
});
104102
</script>
105-
`;
106-
add_autofixers_issues(content, code, 5);
103+
`);
107104

108105
expect(content.suggestions).toContain(
109106
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
110107
);
111108
});
112109
});
110+
111+
describe.each([{ method: 'set' }, { method: 'update' }])(
112+
'set_or_update_state ($method)',
113+
({ method }) => {
114+
it(`should add suggestions when using .${method}() on a stateful variable with a literal init`, () => {
115+
const content = run_autofixers_on_code(`
116+
<script>
117+
const count = $state(0);
118+
function update_count() {
119+
count.${method}(43);
120+
}
121+
</script>`);
122+
123+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
124+
expect(content.suggestions).toContain(
125+
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them.`,
126+
);
127+
});
128+
129+
it(`should add suggestions when using .${method}() on a stateful variable with an array init`, () => {
130+
const content = run_autofixers_on_code(`
131+
<script>
132+
const count = $state([0]);
133+
function update_count() {
134+
count.${method}([1]);
135+
}
136+
</script>`);
137+
138+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
139+
expect(content.suggestions).toContain(
140+
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them.`,
141+
);
142+
});
143+
144+
it(`should add suggestions when using .${method}() on a stateful variable with conditional if it's not sure if the method could actually be present on the variable ($state({}))`, () => {
145+
const content = run_autofixers_on_code(`
146+
<script>
147+
const count = $state({ value: 0 });
148+
function update_count() {
149+
count.${method}({ value: 43 });
150+
}
151+
</script>`);
152+
153+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
154+
expect(content.suggestions).toContain(
155+
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
156+
);
157+
});
158+
159+
it(`should add suggestions when using .${method}() on a stateful variable with conditional if it's not sure if the method could actually be present on the variable ($state(new Class()))`, () => {
160+
const content = run_autofixers_on_code(`
161+
<script>
162+
const count = $state(new Class());
163+
function update_count() {
164+
count.${method}(new Class());
165+
}
166+
</script>`);
167+
168+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
169+
expect(content.suggestions).toContain(
170+
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
171+
);
172+
});
173+
174+
it(`should add suggestions when using .${method}() on a stateful variable with conditional if it's not sure if the method could actually be present on the variable ($state(variable_name))`, () => {
175+
const content = run_autofixers_on_code(`
176+
<script>
177+
const { init } = $props();
178+
const count = $state(init);
179+
function update_count() {
180+
count.${method}(43);
181+
}
182+
</script>`);
183+
184+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
185+
expect(content.suggestions).toContain(
186+
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
187+
);
188+
});
189+
190+
it(`should not add suggestions when using .${method} on a stateful variable if it's not a method call`, () => {
191+
const content = run_autofixers_on_code(`
192+
<script>
193+
const count = $state({});
194+
function update_count() {
195+
console.log(count.${method});
196+
}
197+
</script>`);
198+
199+
expect(content.suggestions).not.toContain(
200+
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
201+
);
202+
});
203+
},
204+
);
205+
206+
describe('imported_runes', () => {
207+
describe.each([{ source: 'svelte' }, { source: 'svelte/runes' }])(
208+
'from "$source"',
209+
({ source }) => {
210+
describe.each(dollarless_runes)('single import ($rune)', ({ rune }) => {
211+
it(`should add suggestions when importing '${rune}' from '${source}'`, () => {
212+
const content = run_autofixers_on_code(`
213+
<script>
214+
import { ${rune} } from '${source}';
215+
</script>`);
216+
217+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
218+
expect(content.suggestions).toContain(
219+
`You are importing "${rune}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${rune}" directly.`,
220+
);
221+
});
222+
223+
it(`should add suggestions when importing "${rune}" as the default export from '${source}'`, () => {
224+
const content = run_autofixers_on_code(`
225+
<script>
226+
import ${rune} from '${source}';
227+
</script>`);
228+
229+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
230+
expect(content.suggestions).toContain(
231+
`You are importing "${rune}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${rune}" directly.`,
232+
);
233+
});
234+
235+
it(`should add suggestions when importing '${rune}' as the namespace export from '${source}'`, () => {
236+
const content = run_autofixers_on_code(`
237+
<script>
238+
import * as ${rune} from '${source}';
239+
</script>`);
240+
241+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
242+
expect(content.suggestions).toContain(
243+
`You are importing "${rune}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${rune}" directly.`,
244+
);
245+
});
246+
});
247+
248+
it(`should add suggestions when importing multiple runes from '${source}'`, () => {
249+
const content = run_autofixers_on_code(`
250+
<script>
251+
import { onMount, state, effect } from '${source}';
252+
</script>`);
253+
254+
expect(content.suggestions.length).toBeGreaterThanOrEqual(2);
255+
expect(content.suggestions).toContain(
256+
`You are importing "state" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$state" directly.`,
257+
);
258+
expect(content.suggestions).toContain(
259+
`You are importing "effect" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$effect" directly.`,
260+
);
261+
});
262+
263+
it(`should not add suggestions when importing other identifiers from '${source}'`, () => {
264+
const content = run_autofixers_on_code(`
265+
<script>
266+
import { onMount } from '${source}';
267+
</script>`);
268+
269+
expect(content.suggestions).not.toContain(
270+
`You are importing "onMount" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$onMount" directly.`,
271+
);
272+
});
273+
},
274+
);
275+
});
276+
277+
describe('derived_with_function', () => {
278+
it(`should add suggestions when using a function as the first argument to $derived`, () => {
279+
const content = run_autofixers_on_code(`
280+
<script>
281+
const value = $derived(() => {
282+
return 43;
283+
});
284+
</script>`);
285+
286+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
287+
expect(content.suggestions).toContain(
288+
'You are passing a function to $derived when declaring "value" but $derived expects an expression. You can use $derived.by instead.',
289+
);
290+
});
291+
292+
it(`should add suggestions when using a function as the first argument to $derived in classes`, () => {
293+
const content = run_autofixers_on_code(`
294+
<script>
295+
class Double {
296+
value = $derived(() => 43);
297+
}
298+
</script>`);
299+
300+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
301+
expect(content.suggestions).toContain(
302+
'You are passing a function to $derived when declaring "value" but $derived expects an expression. You can use $derived.by instead.',
303+
);
304+
});
305+
306+
it(`should add suggestions when using a function as the first argument to $derived in classes constructors`, () => {
307+
const content = run_autofixers_on_code(`
308+
<script>
309+
class Double {
310+
value;
311+
312+
constructor(){
313+
this.value = $derived(function() { return 44; });
314+
}
315+
}
316+
</script>`);
317+
318+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
319+
expect(content.suggestions).toContain(
320+
'You are passing a function to $derived when declaring "value" but $derived expects an expression. You can use $derived.by instead.',
321+
);
322+
});
323+
324+
it(`should add suggestions when using a function as the first argument to $derived without the declaring part if it's not an identifier`, () => {
325+
const content = run_autofixers_on_code(`
326+
<script>
327+
const { destructured } = $derived(() => 43);
328+
</script>`);
329+
330+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
331+
expect(content.suggestions).toContain(
332+
'You are passing a function to $derived but $derived expects an expression. You can use $derived.by instead.',
333+
);
334+
});
335+
336+
it(`should add suggestions when using a function as the first argument to $derived.by`, () => {
337+
const content = run_autofixers_on_code(`
338+
<script>
339+
const { destructured } = $derived.by(() => 43);
340+
</script>`);
341+
342+
expect(content.suggestions).not.toContain(
343+
'You are passing a function to $derived but $derived expects an expression. You can use $derived.by instead.',
344+
);
345+
});
346+
});
113347
});

0 commit comments

Comments
 (0)