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

* should not extend scope for across slots

* disallow named slots inheriting let: scope from default slots

* fix tests

* fix test

* fix

* add runtime tests

* rename test since it doesn't inherit anymore

* fix lint

* remove warnings

* add compile script

* document script

* improve warning

* fix test

* handle renames

* fix lint

* gather names from all parents instead of just the nearest

* remove unused import

* add reminder

---------

Co-authored-by: gtmnayan <gtmnayan@gmail.com>
pull/8654/head svelte@4.0.0-next.0
Tan Li Hau 2 years ago committed by GitHub
parent 198dbcf714
commit 5c6d111065
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -0,0 +1,40 @@
// Compile all Svelte files in a directory to JS and CSS files
// Usage: node scripts/compile-test.js <directory>
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]);
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, '');
let w;
for (const [name, opts] of options) {
const dir = `${cwd}/_output/${name}`;
const { js, css, warnings } = compile(contents, {
...opts,
filename: file
});
if (warnings.length) {
w = warnings;
}
mkdirSync(dir, { recursive: true });
js.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.js')}`, js.code);
css.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.css')}`, css.code);
}
if (w) {
console.log(`Warnings for ${file}:`);
console.log(w);
}
}

@ -1643,8 +1643,9 @@ export default class Component {
* @param {string} name
* @param {any} node
* @param {import('./nodes/shared/TemplateScope.js').default} template_scope
* @param {import("./nodes/shared/Node.js").default} [owner]
*/
warn_if_undefined(name, node, template_scope) {
warn_if_undefined(name, node, template_scope, owner) {
if (name[0] === '$') {
if (name === '$' || (name[1] === '$' && !is_reserved_keyword(name))) {
return this.error(node, compiler_errors.illegal_global(name));
@ -1656,6 +1657,34 @@ export default class Component {
if (this.var_lookup.has(name) && !this.var_lookup.get(name).global) return;
if (template_scope && template_scope.names.has(name)) return;
if (globals.has(name) && node.type !== 'InlineComponent') return;
function has_out_of_scope_let() {
for (let parent = owner.parent; parent; parent = parent.parent) {
if (parent.type === 'InlineComponent') {
const { let_attributes } = parent;
for (const attr of let_attributes) {
if (
// @ts-expect-error
// TODO extract_names only considers patterns but let attributes return expressions
(attr.expression && extract_names(attr.expression).includes(name)) ||
attr.name === name
)
return true;
}
}
}
return false;
}
if (owner && has_out_of_scope_let()) {
return this.warn(node, {
code: 'missing-declaration',
message: `let:${name} declared on parent component cannot be used inside named slot`
});
}
this.warn(node, compiler_warnings.missing_declaration(name, !!this.ast.instance));
}

@ -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';
@ -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 = [];
@ -37,6 +33,8 @@ export default class InlineComponent extends Node {
/** @type {string} */
namespace;
/** @type {Attribute[]} */
let_attributes;
/**
* @param {import('../Component.js').default} component
* @param {import('./shared/Node.js').default} parent
@ -58,6 +56,8 @@ export default class InlineComponent extends Node {
this.name === 'svelte:component'
? new Expression(component, this, scope, info.expression)
: null;
const let_attributes = (this.let_attributes = []);
info.attributes.forEach(
/** @param {import('../../interfaces.js').BaseDirective | import('../../interfaces.js').Attribute | import('../../interfaces.js').SpreadAttribute} node */ (
node
@ -84,7 +84,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);
@ -98,21 +98,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.handlers.forEach(
/** @param {any} handler */ (handler) => {
handler.modifiers.forEach(
@ -178,6 +166,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() {

@ -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();

@ -77,6 +77,7 @@ export default class Expression {
this.scope_map = map;
const expression = this;
let function_expression;
// discover dependencies, but don't change the code yet
walk(info, {
/**
@ -125,7 +126,7 @@ export default class Expression {
dependencies.add(name);
}
component.add_reference(node, name);
component.warn_if_undefined(name, nodes[0], template_scope);
component.warn_if_undefined(name, nodes[0], template_scope, owner);
}
this.skip();
}
@ -376,8 +377,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}
@ -458,6 +460,7 @@ export default class Expression {
}
}
});
if (declarations.length > 0) {
block.maintain_context = true;
declarations.forEach(

@ -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;
@ -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(

@ -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)

@ -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()

@ -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)) {

@ -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}

@ -0,0 +1,7 @@
export default {
html: `
<div>0 - undefined</div>
<div>1 - undefined</div>
<div>2 - undefined</div>
`
};

@ -0,0 +1,7 @@
<script>
import Nested from './Nested.svelte';
</script>
<Nested let:item let:value>
<div>{item} - {value}</div>
</Nested>

@ -0,0 +1,3 @@
<slot />
<slot thing={2} name="thing"/>

@ -0,0 +1,3 @@
export default {
html: '<span>undefined</span><span>2</span>'
};

@ -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>

@ -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>
`,
@ -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>
`

@ -1,5 +1,6 @@
<script>
import Nested from './Nested.svelte';
let count = 42;
</script>
<Nested let:count>

@ -0,0 +1,3 @@
export default {
error: 'thing is not defined'
};

@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';
</script>
<Nested let:thing>
<svelte:fragment slot="thing"><span>{thing}</span></svelte:fragment>
</Nested>

@ -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>

@ -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>

@ -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>

@ -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>

@ -2,8 +2,6 @@ export default {
html: `
<span>1</span>
<span>1</span>
<span>1</span>
<span>1</span>
`,
async test({ assert, target, component }) {
@ -14,8 +12,6 @@ export default {
`
<span>2</span>
<span>2</span>
<span>2</span>
<span>2</span>
`
);
}

@ -8,9 +8,3 @@
<span>{reflected}</span>
</svelte:fragment>
</A>
<A {x} let:reflected>
<svelte:fragment slot="main">
<span>{reflected}</span>
</svelte:fragment>
</A>

@ -0,0 +1,27 @@
<script>
import Nested from './Nested.svelte';
</script>
<Nested let:count>
<p>
count in default slot: {count}
</p>
<p slot="bar">
count in bar slot: {count}
</p>
</Nested>
<Nested let:count>
<p>
count in default slot: {count}
</p>
<p>
count in bar slot: {count}
</p>
</Nested>
<Nested let:foo={count}>
<p slot="bar">
count in bar slot: {count}
</p>
</Nested>

@ -0,0 +1,26 @@
[
{
"code": "missing-declaration",
"message": "let:count declared on parent component cannot be used inside named slot",
"start": {
"column": 22,
"line": 10
},
"end": {
"column": 27,
"line": 10
}
},
{
"code": "missing-declaration",
"message": "let:count declared on parent component cannot be used inside named slot",
"start": {
"column": 22,
"line": 25
},
"end": {
"column": 27,
"line": 25
}
}
]
Loading…
Cancel
Save