Merge pull request #2096 from sveltejs/gh-2024

handle implicit and explicit-but-undefined props

Author: Rich-Harris

src/compile/Component.ts

@@ -28,6 +28,7 @@ type ComponentOptions = {
 	immutable?: boolean;
 	props?: string;
 	props_object?: string;
+	props_node?: Node;
 };
 
 // We need to tell estree-walker that it should always
@@ -131,7 +132,25 @@ export default class Component {
 		if (this.componentOptions.props) {
 			this.has_reactive_assignments = true;
 
-			const variable = this.var_lookup.get(this.componentOptions.props_object);
+			const name = this.componentOptions.props_object;
+
+			if (!this.ast.module && !this.ast.instance) {
+				this.add_var({
+					name,
+					export_name: name,
+					implicit: true
+				});
+			}
+
+			const variable = this.var_lookup.get(name);
+
+			if (!variable) {
+				this.error(this.componentOptions.props_node, {
+					code: 'missing-declaration',
+					message: `'${name}' is not defined`
+				});
+			}
+
 			variable.reassigned = true;
 		}
 
@@ -1229,6 +1248,7 @@ function process_component_options(component: Component, nodes) {
 				const { name } = flattenReference(attribute.expression);
 
 				componentOptions.props = `[✂${start}-${end}✂]`;
+				componentOptions.props_node = attribute.expression;
 				componentOptions.props_object = name;
 			}
 

src/compile/render-dom/index.ts

@@ -258,11 +258,13 @@ export default function dom(
 		${component.fully_hoisted.length > 0 && component.fully_hoisted.join('\n\n')}
 	`);
 
-	const filtered_declarations = component.vars.filter(variable => {
-		return (variable.referenced || variable.export_name) && !variable.hoistable;
-	}).map(variable => variable.name);
+	const filtered_declarations = component.vars
+		.filter(v => ((v.referenced || v.export_name) && !v.hoistable))
+		.map(v => v.name);
 
 	const filtered_props = props.filter(prop => {
+		if (prop.name === component.componentOptions.props_object) return false;
+
 		const variable = component.var_lookup.get(prop.name);
 
 		if (variable.hoistable) return false;
@@ -284,6 +286,7 @@ export default function dom(
 	const has_definition = (
 		component.javascript ||
 		filtered_props.length > 0 ||
+		component.componentOptions.props_object ||
 		component.partly_hoisted.length > 0 ||
 		filtered_declarations.length > 0 ||
 		component.reactive_declarations.length > 0
@@ -299,8 +302,11 @@ export default function dom(
 	});
 
 	const user_code = component.javascript || (
-		!component.ast.instance && !component.ast.module && filtered_props.length > 0
-			? `let { ${filtered_props.map(x => x.name).join(', ')} } = $$props;`
+		!component.ast.instance && !component.ast.module && (filtered_props.length > 0 || component.componentOptions.props)
+			? [
+				component.componentOptions.props && `let ${component.componentOptions.props} = $$props;`,
+				filtered_props.length > 0 && `let { ${filtered_props.map(x => x.name).join(', ')} } = $$props;`
+			].filter(Boolean).join('\n')
 			: null
 	);
 

src/compile/render-ssr/index.ts

@@ -24,14 +24,17 @@ export default function ssr(
 
 	let user_code;
 
-	// TODO remove this, just use component.symbols everywhere
-	const props = component.vars.filter(variable => !variable.module && variable.export_name);
+	// TODO remove this, just use component.vars everywhere
+	const props = component.vars.filter(variable => !variable.module && variable.export_name && variable.export_name !== component.componentOptions.props_object);
 
 	if (component.javascript) {
 		component.rewrite_props();
 		user_code = component.javascript;
-	} else if (!component.ast.instance && !component.ast.module && props.length > 0) {
-		user_code = `let { ${props.map(prop => prop.export_name).join(', ')} } = $$props;`
+	} else if (!component.ast.instance && !component.ast.module && (props.length > 0 || component.componentOptions.props)) {
+		user_code = [
+			component.componentOptions.props && `let ${component.componentOptions.props} = $$props;`,
+			props.length > 0 && `let { ${props.map(prop => prop.export_name).join(', ')} } = $$props;`
+		].filter(Boolean).join('\n');
 	}
 
 	const reactive_stores = component.vars.filter(variable => variable.name[0] === '$');

test/runtime/samples/props-implicit/_config.js

@@ -0,0 +1,17 @@
+export default {
+	props: {
+		x: 1
+	},
+
+	html: `
+		<pre>{"x":1}</pre>
+	`,
+
+	async test({ assert, component, target }) {
+		await component.$set({ x: 2 });
+
+		assert.htmlEqual(target.innerHTML, `
+			<pre>{"x":2}</pre>
+		`);
+	}
+};

test/runtime/samples/props-implicit/main.svelte

@@ -0,0 +1,3 @@
+<svelte:options bind:props={foo}/>
+
+<pre>{JSON.stringify(foo)}</pre>

test/runtime/samples/props-undeclared/_config.js

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

test/runtime/samples/props-undeclared/main.svelte

@@ -0,0 +1,5 @@
+<script></script>
+
+<svelte:options bind:props={foo}/>
+
+<pre>{JSON.stringify(foo)}</pre>