Skip to content

Commit c7060c8

Browse files
paoloricciutipaoloricciuti
authored
Merge pull request #20 from sveltejs/attachments-autofixer
2 parents ef2d569 + 8414ffb commit c7060c8

File tree

3 files changed

+221
-0
lines changed

3 files changed

+221
-0
lines changed

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

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,4 +514,177 @@ describe('add_autofixers_issues', () => {
514514
);
515515
});
516516
});
517+
518+
describe('suggest_attachments', () => {
519+
describe('bind:this', () => {
520+
it('should add suggestions when using bind:this on an element', () => {
521+
const content = run_autofixers_on_code(`
522+
<script>
523+
let a = $state();
524+
</script>
525+
526+
<a bind:this={a} />`);
527+
528+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
529+
expect(content.suggestions).toContain(
530+
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
531+
);
532+
});
533+
534+
it('should not add suggestions when using bind:this on a component', () => {
535+
const content = run_autofixers_on_code(`
536+
<script>
537+
import Child from './Child.svelte';
538+
let a = $state();
539+
</script>
540+
541+
<Child bind:this={a} />`);
542+
543+
expect(content.suggestions).not.toContain(
544+
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
545+
);
546+
});
547+
548+
it('should not add suggestions when using bind:this on a component nested in an element', () => {
549+
const content = run_autofixers_on_code(`
550+
<script>
551+
import Child from './Child.svelte';
552+
let a = $state();
553+
</script>
554+
555+
<div>
556+
<Child bind:this={a} />
557+
</div>`);
558+
559+
expect(content.suggestions).not.toContain(
560+
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
561+
);
562+
});
563+
564+
it('should add suggestions but not suggest attachments when using bind:this on an element and the desired svelte version is 4', () => {
565+
const content = run_autofixers_on_code(
566+
`
567+
<script>
568+
let a;
569+
</script>
570+
571+
<a bind:this={a} />`,
572+
4,
573+
);
574+
575+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
576+
expect(content.suggestions).toContain(
577+
'The usage of `bind:this` can often be replaced with an easier to read `action`. Consider using the latter if possible.',
578+
);
579+
});
580+
});
581+
582+
describe('use:', () => {
583+
it('should add suggestions when using use: on an element and the action is declared as a function', () => {
584+
const content = run_autofixers_on_code(
585+
`<script>
586+
function my_action(node) {
587+
// do something with the node
588+
}
589+
</script>
590+
591+
<a use:my_action />`,
592+
);
593+
594+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
595+
expect(content.suggestions).toContain(
596+
'Consider using an `attachment` instead of an `action` for "my_action".',
597+
);
598+
});
599+
600+
it('should add suggestions when using use: on an element and the action is declared as a variable', () => {
601+
const content = run_autofixers_on_code(
602+
`<script>
603+
const my_action = (node) => {
604+
// do something with the node
605+
}
606+
</script>
607+
608+
<a use:my_action />`,
609+
);
610+
611+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
612+
expect(content.suggestions).toContain(
613+
'Consider using an `attachment` instead of an `action` for "my_action".',
614+
);
615+
});
616+
617+
it('should add suggestions when using use: on an element and the action is declared as an object', () => {
618+
const content = run_autofixers_on_code(
619+
`<script>
620+
const my_action = {
621+
action: (node) => {
622+
// do something with the node
623+
}
624+
};
625+
</script>
626+
627+
<a use:my_action.action />`,
628+
);
629+
630+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
631+
expect(content.suggestions).toContain(
632+
'Consider using an `attachment` instead of an `action` for "my_action".',
633+
);
634+
});
635+
636+
it('should not add suggestions when using use: on an element and the desired svelte version is 4', () => {
637+
const content = run_autofixers_on_code(
638+
`<script>
639+
function my_action(node) {
640+
// do something with the node
641+
}
642+
</script>
643+
644+
<a use:my_action />`,
645+
4,
646+
);
647+
648+
expect(content.suggestions).not.toContain(
649+
'Consider using an `attachment` instead of an `action` for "my_action".',
650+
);
651+
});
652+
653+
it('should not add suggestions when using use: on an element and the action comes from an import', () => {
654+
const content = run_autofixers_on_code(
655+
`<script>
656+
import { my_action } from './actions.js';
657+
</script>
658+
659+
<a use:my_action />`,
660+
);
661+
662+
expect(content.suggestions).not.toContain(
663+
'Consider using an `attachment` instead of an `action` for "my_action".',
664+
);
665+
});
666+
667+
it('should not add suggestions when using use: on an element and the action comes from the props', () => {
668+
const content = run_autofixers_on_code(
669+
`<script>
670+
const { my_action } = $props();
671+
</script>
672+
673+
<a use:my_action />`,
674+
);
675+
676+
expect(content.suggestions).not.toContain(
677+
'Consider using an `attachment` instead of an `action` for "my_action".',
678+
);
679+
});
680+
681+
it('should not add suggestions when using use: on an element and the action comes from a global variable', () => {
682+
const content = run_autofixers_on_code(`<a use:my_action />`);
683+
684+
expect(content.suggestions).not.toContain(
685+
'Consider using an `attachment` instead of an `action` for "my_action".',
686+
);
687+
});
688+
});
689+
});
517690
});

‎packages/mcp-server/src/mcp/autofixers/visitors/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ export * from './wrong-property-access-state.js';
1616
export * from './imported-runes.js';
1717
export * from './derived-with-function.js';
1818
export * from './use-runes-instead-of-store.js';
19+
export * from './suggest-attachments.js';

‎packages/mcp-server/src/mcp/autofixers/visitors/suggest-attachments.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import type { Identifier } from 'estree';
2+
import type { Autofixer } from './index.js';
3+
import { left_most_id } from '../ast/utils.js';
4+
5+
export const suggest_attachments: Autofixer = {
6+
SvelteDirective(node, { state, next, path }) {
7+
if (node.kind === 'Binding' && node.key.name.name === 'this') {
8+
const parent_element = path.findLast((p) => p.type === 'SvelteElement');
9+
if (parent_element?.kind === 'html' && parent_element.startTag.attributes.includes(node)) {
10+
let better_an_attachment = ` or even better an \`attachment\``;
11+
if (state.desired_svelte_version === 4) {
12+
better_an_attachment = ``;
13+
}
14+
state.output.suggestions.push(
15+
`The usage of \`bind:this\` can often be replaced with an easier to read \`action\`${better_an_attachment}. Consider using the latter if possible.`,
16+
);
17+
}
18+
} else if (node.kind === 'Action' && state.desired_svelte_version === 5) {
19+
let id: Identifier | null = null;
20+
if (node.key.name.type === 'Identifier') {
21+
id = node.key.name;
22+
} else if (node.key.name.type === 'MemberExpression') {
23+
id = left_most_id(node.key.name);
24+
}
25+
if (id) {
26+
const reference = state.parsed.find_reference_by_id(id);
27+
const definition = reference?.resolved?.defs[0];
28+
console.log(definition);
29+
if (
30+
definition &&
31+
(definition.type === 'Variable' ||
32+
!(definition.type === 'ImportBinding' || definition.type === 'Parameter')) &&
33+
!(
34+
definition.type === 'Variable' &&
35+
definition.node.init?.type === 'CallExpression' &&
36+
state.parsed.is_rune(definition.node.init, ['$props'])
37+
)
38+
) {
39+
state.output.suggestions.push(
40+
`Consider using an \`attachment\` instead of an \`action\` for "${id.name}".`,
41+
);
42+
}
43+
}
44+
}
45+
next();
46+
},
47+
};

0 commit comments

Comments
 (0)