Merge pull request #41 from 43081j/effect-pre-assign

fix: check effect.pre in assign-in-effect

Author: paoloricciuti

.changeset/cool-tips-explain.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/mcp': patch
+---
+
+fix: check effect.pre in assign-in-effect

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

@@ -61,7 +61,7 @@ describe('add_autofixers_issues', () => {
 				<script>
 					const count = ${init}(0);
 				</script>
-				
+
 				<button onclick={() => count = 43}>Increment</button>
 				`);
 
@@ -74,7 +74,7 @@ describe('add_autofixers_issues', () => {
 				const content = run_autofixers_on_code(`
 				<script>
 					const count = 0;
-					
+
 					$effect(() => {
 						count = 43;
 					});
@@ -89,7 +89,7 @@ describe('add_autofixers_issues', () => {
 				const content = run_autofixers_on_code(`
 				<script>
 					let count = ${init}(0);
-					
+
 					$effect(() => {
 						count++;
 					});
@@ -105,7 +105,7 @@ describe('add_autofixers_issues', () => {
 				const content = run_autofixers_on_code(`
 				<script>
 					let count = ${init}({ value: 0 });
-					
+
 					$effect(() => {
 						count.value = 42;
 					});
@@ -116,6 +116,22 @@ describe('add_autofixers_issues', () => {
 					'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
 				);
 			});
+
+			it(`should add a suggestion for variables that are mutated within an effect.pre`, () => {
+				const content = run_autofixers_on_code(`
+				<script>
+					let count = ${init}({ value: 0 });
+
+					$effect.pre(() => {
+						count.value = 42;
+					});
+				</script>
+				`);
+
+				expect(content.suggestions).toContain(
+					'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
+				);
+			});
 		});
 	});
 

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

@@ -11,19 +11,11 @@ function run_if_in_effect(
 ) {
 	const in_effect = path.findLast(
 		(node) =>
-			node.type === 'CallExpression' &&
-			node.callee.type === 'Identifier' &&
-			node.callee.name === '$effect',
+			node.type === 'CallExpression' && state.parsed.is_rune(node, ['$effect', '$effect.pre']),
 	);
 
-	if (
-		in_effect &&
-		in_effect.type === 'CallExpression' &&
-		(in_effect.callee.type === 'Identifier' || in_effect.callee.type === 'MemberExpression')
-	) {
-		if (state.parsed.is_rune(in_effect, ['$effect', '$effect.pre'])) {
-			to_run();
-		}
+	if (in_effect) {
+		to_run();
 	}
 }