Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

complain if named slots other than direct descendant of component #4509

Merged
merged 7 commits into from
Mar 15, 2020
Merged

complain if named slots other than direct descendant of component #4509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d185a20
complain if named slots other than direct descendant of component
tanhauhau Mar 5, 2020
b43cf2d
adjust error message
Conduitry Mar 8, 2020
6f634b8
remove if/each slot special check
tanhauhau Mar 9, 2020
45632d0
allow slot not to be top level if it is within a custom element
tanhauhau Mar 9, 2020
cea1193
update error message
tanhauhau Mar 15, 2020
af13983
Merge branch 'master' into pr/4509
Conduitry Mar 15, 2020
4b1002f
update changelog
Conduitry Mar 15, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Allow `<svelte:self>` to be used in a slot ([#2798](https://github.com/sveltejs/svelte/issues/2798))
* Expose object of unknown props in `$$restProps` ([#2930](https://github.com/sveltejs/svelte/issues/2930))
* Prevent passing named slots other than from the top level within a component ([#3385](https://github.com/sveltejs/svelte/issues/3385))
* Allow transitions and animations to work within iframes ([#3624](https://github.com/sveltejs/svelte/issues/3624))
* Fix initialising slot fallbacks when unnecessary ([#3763](https://github.com/sveltejs/svelte/issues/3763))
* Disallow binding directly to `const` variables ([#4479](https://github.com/sveltejs/svelte/issues/4479))
Expand Down
31 changes: 12 additions & 19 deletions src/compiler/compile/nodes/Element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export default class Element extends Node {
}

validate_attributes() {
const { component } = this;
const { component, parent } = this;

const attribute_map = new Map();

Expand Down Expand Up @@ -395,26 +395,10 @@ export default class Element extends Node {
component.slot_outlets.add(name);
}

let ancestor = this.parent;
do {
if (ancestor.type === 'InlineComponent') break;
if (ancestor.type === 'Element' && /-/.test(ancestor.name)) break;

if (ancestor.type === 'IfBlock' || ancestor.type === 'EachBlock') {
const type = ancestor.type === 'IfBlock' ? 'if' : 'each';
const message = `Cannot place slotted elements inside an ${type}-block`;

component.error(attribute, {
code: `invalid-slotted-content`,
message
});
}
} while (ancestor = ancestor.parent);

if (!ancestor) {
if (!(parent.type === 'InlineComponent' || within_custom_element(parent))) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can actually allow it if it is within custom element, since we are going to create them just like a normal element, browser will figure out itself

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this relates to #1689 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the error message be adjusted then? Something like 'must be a direct child of a component or a descendant of a custom element'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup! updated the error message

component.error(attribute, {
code: `invalid-slotted-content`,
message: `Element with a slot='...' attribute must be a descendant of a component or custom element`
message: `Element with a slot='...' attribute must be a child of a component or a descendant of a custom element`,
});
}
}
Expand Down Expand Up @@ -776,3 +760,12 @@ function should_have_attribute(
message: `A11y: <${name}> element should have ${article} ${sequence} attribute`
});
}

function within_custom_element(parent: INode) {
while (parent) {
if (parent.type === 'InlineComponent') return false;
if (parent.type === 'Element' && /-/.test(parent.name)) return true;
parent = parent.parent;
}
return false;
}
1 change: 1 addition & 0 deletions test/runtime/samples/component-slot-nested-error-2/Nested.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<slot name="slot2"></slot>
3 changes: 3 additions & 0 deletions test/runtime/samples/component-slot-nested-error-2/_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
error: [`Element with a slot='...' attribute must be a child of a component or a descendant of a custom element`]
};
11 changes: 11 additions & 0 deletions test/runtime/samples/component-slot-nested-error-2/main.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
import Nested from "./Nested.svelte";
</script>

<Nested>
<div slot="slot1">
<div>
<div slot="slot2" />
</div>
</div>
</Nested>
1 change: 1 addition & 0 deletions test/runtime/samples/component-slot-nested-error-3/Nested.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<slot name="slot2"></slot>
3 changes: 3 additions & 0 deletions test/runtime/samples/component-slot-nested-error-3/_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
error: [`Element with a slot='...' attribute must be a child of a component or a descendant of a custom element`]
};
11 changes: 11 additions & 0 deletions test/runtime/samples/component-slot-nested-error-3/main.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
import Nested from "./Nested.svelte";
</script>

<Nested>
<div>
<div>
<div slot="slot2" />
</div>
</div>
</Nested>
1 change: 1 addition & 0 deletions test/runtime/samples/component-slot-nested-error/Nested.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<slot name="slot2"></slot>
3 changes: 3 additions & 0 deletions test/runtime/samples/component-slot-nested-error/_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
error: [`Element with a slot='...' attribute must be a child of a component or a descendant of a custom element`]
};
9 changes: 9 additions & 0 deletions test/runtime/samples/component-slot-nested-error/main.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
import Nested from "./Nested.svelte";
</script>

<Nested>
<div slot="slot1">
<div slot="slot2" />
</div>
</Nested>
9 changes: 9 additions & 0 deletions test/validator/samples/component-slotted-custom-element-2/errors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[
{
"code": "invalid-slotted-content",
"message": "Element with a slot='...' attribute must be a child of a component or a descendant of a custom element",
"start": { "line": 10, "column": 9, "character": 138 },
"end": { "line": 10, "column": 19, "character": 148 },
"pos": 138
}
]
14 changes: 14 additions & 0 deletions test/validator/samples/component-slotted-custom-element-2/input.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<script>
import Nested from './Nested.svelte';
let thing = false;
</script>

<custom-element>
<Nested>
{#if thing}
<div>
<div slot='bar' />
</div>
{/if}
</Nested>
</custom-element>
1 change: 1 addition & 0 deletions test/validator/samples/component-slotted-custom-element/errors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
15 changes: 15 additions & 0 deletions test/validator/samples/component-slotted-custom-element/input.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<script>
import Nested from './Nested.svelte';
let thing = false;
</script>

<Nested>
<custom-element>
<div>
<div slot='foo' />
</div>
{#if thing}
<div slot='bar' />
{/if}
</custom-element>
</Nested>
2 changes: 1 addition & 1 deletion test/validator/samples/component-slotted-each-block/errors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[{
"code": "invalid-slotted-content",
"message": "Cannot place slotted elements inside an each-block",
"message": "Element with a slot='...' attribute must be a child of a component or a descendant of a custom element",
"start": {
"line": 7,
"column": 7,
Expand Down
2 changes: 1 addition & 1 deletion test/validator/samples/component-slotted-if-block/errors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[{
"code": "invalid-slotted-content",
"message": "Cannot place slotted elements inside an if-block",
"message": "Element with a slot='...' attribute must be a child of a component or a descendant of a custom element",
"start": {
"line": 7,
"column": 7,
Expand Down
2 changes: 1 addition & 1 deletion test/validator/samples/slot-attribute-invalid/errors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[{
"code": "invalid-slotted-content",
"message": "Element with a slot='...' attribute must be a descendant of a component or custom element",
"message": "Element with a slot='...' attribute must be a child of a component or a descendant of a custom element",
"start": {
"line": 1,
"column": 5,
Expand Down