Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,36 @@ describe('add_autofixers_issues', () => {
);
});
});

it('should add a suggestion when calling a function inside an effect', () => {
const content = run_autofixers_on_code(`
<script>
import { fetch_data } from './data.js';
$effect(() => {
fetch_data();
});
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are calling the function \`fetch_data\` inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`,
);
});

it('should add a suggestion when calling a function inside an effect (with non identifier callee)', () => {
const content = run_autofixers_on_code(`
<script>
import { fetch_data } from './data.js';
$effect(() => {
fetch_data.fetch();
});
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are calling a function inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`,
);
});
});

with_possible_inits('($init)', ({ init }) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import type { AssignmentExpression, Identifier, Node, UpdateExpression } from 'estree';
import type {
AssignmentExpression,
CallExpression,
Identifier,
Node,
UpdateExpression,
} from 'estree';
import type { Autofixer, AutofixerState } from './index.js';
import { left_most_id } from '../ast/utils.js';
import type { AST } from 'svelte-eslint-parser';
Expand Down Expand Up @@ -27,9 +33,9 @@ function run_if_in_effect(
}
}

function visitor(
function assign_or_update_visitor(
node: UpdateExpression | AssignmentExpression,
{ state, path }: Context<Node | AST.SvelteNode, AutofixerState>,
{ state, path, next }: Context<Node | AST.SvelteNode, AutofixerState>,
) {
run_if_in_effect(path, state, () => {
function check_if_stateful_id(id: Identifier) {
Expand Down Expand Up @@ -58,9 +64,25 @@ function visitor(
}
}
});
next();
}

function call_expression_visitor(
node: CallExpression,
{ state, path, next }: Context<Node | AST.SvelteNode, AutofixerState>,
) {
run_if_in_effect(path, state, () => {
const function_name =
node.callee.type === 'Identifier' ? `the function \`${node.callee.name}\`` : 'a function';
state.output.suggestions.push(
`You are calling ${function_name} inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`,
);
});
next();
}

export const assign_in_effect: Autofixer = {
UpdateExpression: visitor,
AssignmentExpression: visitor,
UpdateExpression: assign_or_update_visitor,
AssignmentExpression: assign_or_update_visitor,
CallExpression: call_expression_visitor,
};