From b1c663a1944b8bcdca22fe008ec1f2a55717dba9 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 15 May 2024 12:33:12 +0200 Subject: [PATCH 1/3] fix: warn on implicit children snipett shadowing a prop --- .changeset/empty-worms-leave.md | 5 +++++ .../src/compiler/phases/2-analyze/validation.js | 11 +++++++++++ .../errors.json | 14 ++++++++++++++ .../input.svelte | 7 +++++++ 4 files changed, 37 insertions(+) create mode 100644 .changeset/empty-worms-leave.md create mode 100644 packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json create mode 100644 packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/input.svelte diff --git a/.changeset/empty-worms-leave.md b/.changeset/empty-worms-leave.md new file mode 100644 index 000000000000..888aaedb83c9 --- /dev/null +++ b/.changeset/empty-worms-leave.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: warn on implicit children snipett shadowing a prop diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 2c0296d69b48..b1b03d7fefc3 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -39,6 +39,17 @@ import { a11y_validators } from './a11y.js'; * @param {import('zimmerframe').Context} context */ function validate_component(node, context) { + const has_implicit_children = node.fragment.nodes.length > 0; + if ( + has_implicit_children && + node.attributes.some( + (attribute) => + (attribute.type === 'Attribute' || attribute.type === 'BindDirective') && + attribute.name === 'children' + ) + ) { + e.snippet_shadowing_prop(node, 'children'); + } for (const attribute of node.attributes) { if ( attribute.type !== 'Attribute' && diff --git a/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json b/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json new file mode 100644 index 000000000000..f72d7d50567a --- /dev/null +++ b/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "snippet_shadowing_prop", + "message": "This snippet is shadowing the prop `children` with the same name", + "start": { + "column": 0, + "line": 5 + }, + "end": { + "column": 12, + "line": 7 + } + } +] diff --git a/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/input.svelte b/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/input.svelte new file mode 100644 index 000000000000..fb775d090ac0 --- /dev/null +++ b/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/input.svelte @@ -0,0 +1,7 @@ + + + + title + \ No newline at end of file From 1b1545ec1775b25dac453e4840eeea2ab81dd541 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 15 May 2024 12:57:44 +0200 Subject: [PATCH 2/3] chore: update messages --- packages/svelte/messages/compile-errors/template.md | 6 +++++- packages/svelte/src/compiler/errors.js | 13 +++++++++++-- .../src/compiler/phases/2-analyze/validation.js | 2 +- .../errors.json | 4 ++-- .../samples/snippet-shadowing-prop/errors.json | 2 +- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/svelte/messages/compile-errors/template.md b/packages/svelte/messages/compile-errors/template.md index 5c7b620125fa..a3f016d960be 100644 --- a/packages/svelte/messages/compile-errors/template.md +++ b/packages/svelte/messages/compile-errors/template.md @@ -240,9 +240,13 @@ > snippets do not support rest parameters; use an array instead +## snippet_shadowing_children_prop + +> The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute. + ## snippet_shadowing_prop -> This snippet is shadowing the prop `%prop%` with the same name +> This snippet is conflicting with the prop `%prop%` with the same name ## style_directive_invalid_modifier diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index ab7197b06b8d..ad13fe566bfc 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -1077,13 +1077,13 @@ export function snippet_invalid_rest_parameter(node) { } /** - * This snippet is shadowing the prop `%prop%` with the same name + * This snippet is conflicting with the prop `%prop%` with the same name * @param {null | number | NodeLike} node * @param {string} prop * @returns {never} */ export function snippet_shadowing_prop(node, prop) { - e(node, "snippet_shadowing_prop", `This snippet is shadowing the prop \`${prop}\` with the same name`); + e(node, "snippet_shadowing_prop", `This snippet is conflicting with the prop \`${prop}\` with the same name`); } /** @@ -1384,4 +1384,13 @@ export function unexpected_reserved_word(node, word) { */ export function void_element_invalid_content(node) { e(node, "void_element_invalid_content", "Void elements cannot have children or closing tags"); +} + +/** + * The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute. + * @param {null | number | NodeLike} node + * @returns {never} + */ +export function snippet_shadowing_children_prop(node) { + e(node, "snippet_shadowing_children_prop", "The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute."); } \ No newline at end of file diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index b1b03d7fefc3..64ac1eafab09 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -48,7 +48,7 @@ function validate_component(node, context) { attribute.name === 'children' ) ) { - e.snippet_shadowing_prop(node, 'children'); + e.snippet_shadowing_children_prop(node); } for (const attribute of node.attributes) { if ( diff --git a/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json b/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json index f72d7d50567a..d5fe36e14170 100644 --- a/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json +++ b/packages/svelte/tests/validator/samples/snippet-shadowing-prop-implicit-children/errors.json @@ -1,7 +1,7 @@ [ { - "code": "snippet_shadowing_prop", - "message": "This snippet is shadowing the prop `children` with the same name", + "code": "snippet_shadowing_children_prop", + "message": "The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute.", "start": { "column": 0, "line": 5 diff --git a/packages/svelte/tests/validator/samples/snippet-shadowing-prop/errors.json b/packages/svelte/tests/validator/samples/snippet-shadowing-prop/errors.json index c099968091cc..badb069cfdfa 100644 --- a/packages/svelte/tests/validator/samples/snippet-shadowing-prop/errors.json +++ b/packages/svelte/tests/validator/samples/snippet-shadowing-prop/errors.json @@ -1,7 +1,7 @@ [ { "code": "snippet_shadowing_prop", - "message": "This snippet is shadowing the prop `title` with the same name", + "message": "This snippet is conflicting with the prop `title` with the same name", "start": { "column": 1, "line": 6 From 3fa48fbfecbd044530bbaed6a73c3554f5f70d56 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 15 May 2024 13:56:42 +0200 Subject: [PATCH 3/3] chore: regenerate errors --- packages/svelte/src/compiler/errors.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index ad13fe566bfc..8481a836a539 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -1076,6 +1076,15 @@ export function snippet_invalid_rest_parameter(node) { e(node, "snippet_invalid_rest_parameter", "snippets do not support rest parameters; use an array instead"); } +/** + * The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute. + * @param {null | number | NodeLike} node + * @returns {never} + */ +export function snippet_shadowing_children_prop(node) { + e(node, "snippet_shadowing_children_prop", "The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute."); +} + /** * This snippet is conflicting with the prop `%prop%` with the same name * @param {null | number | NodeLike} node @@ -1384,13 +1393,4 @@ export function unexpected_reserved_word(node, word) { */ export function void_element_invalid_content(node) { e(node, "void_element_invalid_content", "Void elements cannot have children or closing tags"); -} - -/** - * The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute. - * @param {null | number | NodeLike} node - * @returns {never} - */ -export function snippet_shadowing_children_prop(node) { - e(node, "snippet_shadowing_children_prop", "The component content is implicitly passed as a `children` snippet which conflicts with the `children` prop set as an attribute."); } \ No newline at end of file