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.

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

Do not expose default slot let bindings to named slots #6049

Merged
merged 19 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 11 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
28 changes: 28 additions & 0 deletions scripts/compile-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { mkdirSync, readFileSync, writeFileSync } from 'fs';
import path from 'path';
import glob from 'tiny-glob/sync.js';
import { compile } from '../src/compiler/index.js';

const cwd = path.resolve(process.argv[2]);
gtm-nayan marked this conversation as resolved.
Show resolved Hide resolved

const options = [
['normal', {}],
['hydrate', { hydratable: true }],
['ssr', { generate: 'ssr' }]
];
for (const file of glob('**/*.svelte', { cwd })) {
const contents = readFileSync(`${cwd}/${file}`, 'utf-8').replace(/\r/g, '');

for (const [name, opts] of options) {
const dir = `${cwd}/_output/${name}`;

const { js, css } = compile(contents, {
...opts,
filename: file
});

mkdirSync(dir, { recursive: true });
js.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.js')}`, js.code);
css.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.css')}`, css.code);
}
}
38 changes: 18 additions & 20 deletions src/compiler/compile/nodes/InlineComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import map_children from './shared/map_children.js';
import Binding from './Binding.js';
import EventHandler from './EventHandler.js';
import Expression from './shared/Expression.js';
import Let from './Let.js';
import compiler_errors from '../compiler_errors.js';
import { regex_only_whitespaces } from '../../utils/patterns.js';

Expand All @@ -22,9 +21,6 @@ export default class InlineComponent extends Node {
/** @type {import('./EventHandler.js').default[]} */
handlers = [];

/** @type {import('./Let.js').default[]} */
lets = [];

/** @type {import('./Attribute.js').default[]} */
css_custom_properties = [];

Expand Down Expand Up @@ -58,6 +54,8 @@ export default class InlineComponent extends Node {
this.name === 'svelte:component'
? new Expression(component, this, scope, info.expression)
: null;

const let_attributes = [];
info.attributes.forEach(
/** @param {any} node */ (node) => {
/* eslint-disable no-fallthrough */
Expand All @@ -82,7 +80,7 @@ export default class InlineComponent extends Node {
this.handlers.push(new EventHandler(component, this, scope, node));
break;
case 'Let':
this.lets.push(new Let(component, this, scope, node));
let_attributes.push(node);
break;
case 'Transition':
return component.error(node, compiler_errors.invalid_transition);
Expand All @@ -94,21 +92,9 @@ export default class InlineComponent extends Node {
/* eslint-enable no-fallthrough */
}
);
if (this.lets.length > 0) {
this.scope = scope.child();
this.lets.forEach(
/** @param {any} l */ (l) => {
const dependencies = new Set([l.name.name]);
l.names.forEach(
/** @param {any} name */ (name) => {
this.scope.add(name, dependencies, this);
}
);
}
);
} else {
this.scope = scope;
}

this.scope = scope;

this.handlers.forEach(
/** @param {any} handler */ (handler) => {
handler.modifiers.forEach(
Expand Down Expand Up @@ -174,6 +160,18 @@ export default class InlineComponent extends Node {
children: info.children
});
}

if (let_attributes.length) {
// copy let: attribute from <Component /> to <svelte:fragment slot="default" />
// as they are for `slot="default"` only
children.forEach((child) => {
const slot = child.attributes.find((attribute) => attribute.name === 'slot');
if (!slot || slot.value[0].data === 'default') {
child.attributes.push(...let_attributes);
}
});
}

this.children = map_children(component, this, this.scope, children);
}
get slot_template_name() {
Expand Down
34 changes: 1 addition & 33 deletions src/compiler/compile/nodes/Slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,39 +40,7 @@ export default class Slot extends Element {
}
);
if (!this.slot_name) this.slot_name = 'default';
if (this.slot_name === 'default') {
// if this is the default slot, add our dependencies to any
// other slots (which inherit our slot values) that were
// previously encountered
component.slots.forEach(
/** @param {any} slot */ (slot) => {
this.values.forEach(
/**
* @param {any} attribute
* @param {any} name
*/ (attribute, name) => {
if (!slot.values.has(name)) {
slot.values.set(name, attribute);
}
}
);
}
);
} else if (component.slots.has('default')) {
// otherwise, go the other way — inherit values from
// a previously encountered default slot
const default_slot = component.slots.get('default');
default_slot.values.forEach(
/**
* @param {any} attribute
* @param {any} name
*/ (attribute, name) => {
if (!this.values.has(name)) {
this.values.set(name, attribute);
}
}
);
}

component.slots.set(this.slot_name, this);
this.cannot_use_innerhtml();
this.not_static_content();
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/compile/nodes/shared/Expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,9 @@ export default class Expression {
node = node.parent;
}
const func_expression = func_declaration[0];
if (node.type === 'InlineComponent' || node.type === 'SlotTemplate') {
// <Comp let:data />

if (node.type === 'SlotTemplate') {
// <svelte:fragment let:data />
this.replace(func_expression);
} else {
// {#each}, {#await}
Expand Down
28 changes: 11 additions & 17 deletions src/compiler/compile/render_dom/wrappers/InlineComponent/index.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import Wrapper from '../shared/Wrapper.js';
import BindingWrapper from '../Element/Binding.js';
import SlotTemplateWrapper from '../SlotTemplate.js';
import { b, p, x } from 'code-red';
import { extract_ignores_above_node } from '../../../../utils/extract_svelte_ignore.js';
import { sanitize } from '../../../../utils/names.js';
import { namespaces } from '../../../../utils/namespaces.js';
import compiler_warnings from '../../../compiler_warnings.js';
import add_to_set from '../../../utils/add_to_set.js';
import { b, x, p } from 'code-red';
import is_dynamic from '../shared/is_dynamic.js';
import bind_this from '../shared/bind_this.js';
import EventHandler from '../Element/EventHandler.js';
import { extract_names } from 'periscopic';
import mark_each_block_bindings from '../shared/mark_each_block_bindings.js';
import { string_to_member_expression } from '../../../utils/string_to_member_expression.js';
import BindingWrapper from '../Element/Binding.js';
import EventHandler from '../Element/EventHandler.js';
import SlotTemplateWrapper from '../SlotTemplate.js';
import Wrapper from '../shared/Wrapper.js';
import bind_this from '../shared/bind_this.js';
import is_dynamic from '../shared/is_dynamic.js';
import { is_head } from '../shared/is_head.js';
import compiler_warnings from '../../../compiler_warnings.js';
import { namespaces } from '../../../../utils/namespaces.js';
import { extract_ignores_above_node } from '../../../../utils/extract_svelte_ignore.js';
import mark_each_block_bindings from '../shared/mark_each_block_bindings.js';

const regex_invalid_variable_identifier_characters = /[^a-zA-Z_$]/g;

Expand Down Expand Up @@ -77,11 +76,6 @@ export default class InlineComponentWrapper extends Wrapper {
).toLowerCase()
};
if (this.node.children.length) {
this.node.lets.forEach((l) => {
extract_names(l.value || l.name).forEach((name) => {
renderer.add_to_context(name, true);
});
});
this.children = this.node.children.map(
(child) =>
new SlotTemplateWrapper(
Expand Down
5 changes: 1 addition & 4 deletions src/compiler/compile/render_dom/wrappers/SlotTemplate.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ export default class SlotTemplateWrapper extends Wrapper {
type: 'slot'
});
this.renderer.blocks.push(this.block);
const seen = new Set(lets.map((l) => l.name.name));
this.parent.node.lets.forEach((l) => {
if (!seen.has(l.name.name)) lets.push(l);
});

/** @type {import('./InlineComponent/index.js').default} */ (this.parent).set_slot(
slot_template_name,
get_slot_definition(this.block, scope, lets)
Expand Down
5 changes: 0 additions & 5 deletions src/compiler/compile/render_ssr/handlers/Slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ export default function (node, renderer, options) {
: ${result}
`);
if (slot && nearest_inline_component) {
const lets = node.lets;
const seen = new Set(lets.map((l) => l.name.name));
nearest_inline_component.lets.forEach((l) => {
if (!seen.has(l.name.name)) lets.push(l);
});
options.slot_scopes.set(slot, {
input: get_slot_scope(node.lets),
output: renderer.pop()
Expand Down
6 changes: 1 addition & 5 deletions src/compiler/compile/render_ssr/handlers/SlotTemplate.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ export default function (node, renderer, options) {
);
renderer.push();
renderer.render(children, options);
const lets = node.lets;
const seen = new Set(lets.map((l) => l.name.name));
parent_inline_component.lets.forEach((l) => {
if (!seen.has(l.name.name)) lets.push(l);
});

const slot_fragment_content = renderer.pop();
if (!is_empty_template_literal(slot_fragment_content)) {
if (options.slot_scopes.has(node.slot_template_name)) {
Expand Down
11 changes: 11 additions & 0 deletions test/runtime/samples/component-slot-default-in-each/Nested.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
let promise = new Promise(resolve => resolve(10));
</script>

{#each {length: 3} as _, i}
<slot item={i}/>
{/each}

{#await promise then value}
<slot {value}/>
{/await}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default {
html: `
<div>0 - undefined</div>
<div>1 - undefined</div>
<div>2 - undefined</div>
`
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
import Nested from './Nested.svelte';
</script>

<Nested let:item let:value>
<div>{item} - {value}</div>
</Nested>
3 changes: 3 additions & 0 deletions test/runtime/samples/component-slot-let-scope-2/Nested.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<slot />
<slot thing={2} name="thing"/>

3 changes: 3 additions & 0 deletions test/runtime/samples/component-slot-let-scope-2/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
html: '<span>undefined</span><span>2</span>'
};
8 changes: 8 additions & 0 deletions test/runtime/samples/component-slot-let-scope-2/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';
</script>

<Nested let:thing>
<span>{thing}</span>
<svelte:fragment slot="thing" let:thing><span>{thing}</span></svelte:fragment>
</Nested>
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
<script>
import { onDestroy } from 'svelte';

let count = 0;

function increment() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export default {
<div>
<p>count in default slot: 0</p>
<p slot="foo">count in foo slot: 0</p>
<p slot="bar">count in bar slot: 0</p>
<p slot="bar">count in bar slot: 42</p>
<button>+1</button>
</div>
`,
Expand All @@ -19,7 +19,7 @@ export default {
<div>
<p>count in default slot: 1</p>
<p slot="foo">count in foo slot: 1</p>
<p slot="bar">count in bar slot: 1</p>
<p slot="bar">count in bar slot: 42</p>
<button>+1</button>
</div>
`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<script>
import Nested from './Nested.svelte';
let count = 42;
</script>

<Nested let:count>
Expand Down
2 changes: 2 additions & 0 deletions test/runtime/samples/component-slot-let-scope/Nested.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<slot thing={2} name="thing"/>

3 changes: 3 additions & 0 deletions test/runtime/samples/component-slot-let-scope/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
error: 'thing is not defined'
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
};
8 changes: 8 additions & 0 deletions test/runtime/samples/component-slot-let-scope/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';

</script>

<Nested let:thing>
<svelte:fragment slot="thing"><span>{thing}</span></svelte:fragment>
</Nested>
6 changes: 4 additions & 2 deletions test/runtime/samples/component-slot-nested-in-slot/One.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
export let a, b;
</script>

<Two {b} let:two>
<slot name="one" slot="two" one={a} two={two}></slot>
<Two {b}>
<svelte:fragment slot="two" let:two>
<slot name="one" one={a} two={two}></slot>
</svelte:fragment>
</Two>
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
export let things;
</script>

<Nested {things} let:thing={x}>
<svelte:fragment slot="main">
<Nested {things}>
<svelte:fragment slot="main" let:thing={x}>
<span>{x}</span>
</svelte:fragment>
</Nested>
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import Nested from './Nested.svelte';
</script>

<Nested let:count>
<svelte:fragment slot="main">
<Nested>
<svelte:fragment slot="main" let:count>
<span>{count}</span>
</svelte:fragment>
</Nested>
4 changes: 2 additions & 2 deletions test/runtime/samples/component-svelte-fragment-let-e/A.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
export let x;
</script>

<B {x} let:reflected>
<svelte:fragment slot="main">
<B {x}>
<svelte:fragment slot="main" let:reflected>
<span>{reflected}</span>
<slot name="main" {reflected} />
</svelte:fragment>
Expand Down
Loading