Pass hoisted values through to slots (#3124)

* Fixed bug with slot props variables

* dont add hoisted items to context

* alternative fix for #2586

* update slots more conservatively

Author: Rich-Harris

src/compiler/compile/nodes/shared/Expression.ts

@@ -13,6 +13,7 @@ import get_object from '../../utils/get_object';
 import { nodes_match } from '../../../utils/nodes_match';
 import Block from '../../render_dom/Block';
 import { INode } from '../interfaces';
+import is_dynamic from '../../render_dom/wrappers/shared/is_dynamic';
 
 const binary_operators: Record<string, number> = {
 	'**': 15,
@@ -211,10 +212,7 @@ export default class Expression {
 			if (name === '$$props') return true;
 
 			const variable = this.component.var_lookup.get(name);
-			if (!variable) return false;
-
-			if (variable.mutated || variable.reassigned) return true; // dynamic internal state
-			if (!variable.module && variable.writable && variable.export_name) return true; // writable props
+			return is_dynamic(variable);
 		});
 	}
 

src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts

@@ -13,6 +13,7 @@ import create_debugging_comment from '../shared/create_debugging_comment';
 import { get_context_merger } from '../shared/get_context_merger';
 import EachBlock from '../../../nodes/EachBlock';
 import TemplateScope from '../../../nodes/shared/TemplateScope';
+import is_dynamic from '../shared/is_dynamic';
 import bind_this from '../shared/bind_this';
 
 export default class InlineComponentWrapper extends Wrapper {
@@ -161,11 +162,7 @@ export default class InlineComponentWrapper extends Wrapper {
 				const is_let = slot.scope.is_let(name);
 				const variable = renderer.component.var_lookup.get(name);
 
-				if (is_let) fragment_dependencies.add(name);
-
-				if (!variable) return;
-				if (variable.mutated || variable.reassigned) fragment_dependencies.add(name);
-				if (!variable.module && variable.writable && variable.export_name) fragment_dependencies.add(name);
+				if (is_let || is_dynamic(variable)) fragment_dependencies.add(name);
 			});
 		});
 

src/compiler/compile/render_dom/wrappers/Slot.ts

@@ -9,6 +9,7 @@ import add_to_set from '../../utils/add_to_set';
 import get_slot_data from '../../utils/get_slot_data';
 import { stringify_props } from '../../utils/stringify_props';
 import Expression from '../../nodes/shared/Expression';
+import is_dynamic from './shared/is_dynamic';
 
 export default class SlotWrapper extends Wrapper {
 	node: Slot;
@@ -72,17 +73,27 @@ export default class SlotWrapper extends Wrapper {
 			this.node.values.forEach(attribute => {
 				attribute.chunks.forEach(chunk => {
 					if ((chunk as Expression).dependencies) {
-						add_to_set(dependencies, (chunk as Expression).dependencies);
 						add_to_set(dependencies, (chunk as Expression).contextual_dependencies);
+
+						// add_to_set(dependencies, (chunk as Expression).dependencies);
+						(chunk as Expression).dependencies.forEach(name => {
+							const variable = renderer.component.var_lookup.get(name);
+							if (variable && !variable.hoistable) dependencies.add(name);
+						});
 					}
 				});
 
-				if (attribute.dependencies.size > 0) {
-					changes_props.push(`${attribute.name}: ${[...attribute.dependencies].join(' || ')}`);
+				const dynamic_dependencies = Array.from(attribute.dependencies).filter(name => {
+					const variable = renderer.component.var_lookup.get(name);
+					return is_dynamic(variable);
+				});
+
+				if (dynamic_dependencies.length > 0) {
+					changes_props.push(`${attribute.name}: ${dynamic_dependencies.join(' || ')}`);
 				}
 			});
 
-			const arg = dependencies.size > 0 ? `{ ${Array.from(dependencies).join(', ')} }` : '{}';
+			const arg = dependencies.size > 0 ? `{ ${Array.from(dependencies).join(', ')} }` : '';
 
 			renderer.blocks.push(deindent`
 				const ${get_slot_changes} = (${arg}) => (${stringify_props(changes_props)});
@@ -149,8 +160,14 @@ export default class SlotWrapper extends Wrapper {
 			`@transition_out(${slot}, #local);`
 		);
 
-		let update_conditions = [...this.dependencies].map(name => `changed.${name}`).join(' || ');
-		if (this.dependencies.size > 1) update_conditions = `(${update_conditions})`;
+		const dynamic_dependencies = Array.from(this.dependencies).filter(name => {
+			if (name === '$$scope') return true;
+			const variable = renderer.component.var_lookup.get(name);
+			return is_dynamic(variable);
+		});
+
+		let update_conditions = dynamic_dependencies.map(name => `changed.${name}`).join(' || ');
+		if (dynamic_dependencies.length > 1) update_conditions = `(${update_conditions})`;
 
 		block.builders.update.add_block(deindent`
 			if (${slot} && ${slot}.p && ${update_conditions}) {

src/compiler/compile/render_dom/wrappers/shared/is_dynamic.ts

@@ -0,0 +1,10 @@
+import { Var } from '../../../../interfaces';
+
+export default function is_dynamic(variable: Var) {
+	if (variable) {
+		if (variable.mutated || variable.reassigned) return true; // dynamic internal state
+		if (!variable.module && variable.writable && variable.export_name) return true; // writable props
+	}
+
+	return false;
+}

test/runtime/samples/component-slot-let-e/Nested.svelte

@@ -0,0 +1,7 @@
+<script>
+	let fooText = 'foo';
+</script>
+
+<div>
+	<slot someText={fooText}></slot>
+</div>

test/runtime/samples/component-slot-let-e/_config.js

@@ -0,0 +1,20 @@
+export default {
+	html: `
+		<div>
+			<p>foo</p>
+		</div>
+	`,
+
+	async test({ assert, target, window }) {
+		const div = target.querySelector('div');
+		const click = new window.MouseEvent('click');
+
+		await div.dispatchEvent(click);
+
+		assert.htmlEqual(target.innerHTML, `
+			<div>
+				<p>foo</p>
+			</div>
+		`);
+	}
+};

test/runtime/samples/component-slot-let-e/main.svelte

@@ -0,0 +1,7 @@
+<script>
+	import Nested from './Nested.svelte';
+</script>
+
+<Nested let:someText={someText}>
+	<p>{someText}</p>
+</Nested>