Skip to content

Commit

Permalink
Pass hoisted values through to slots (sveltejs#3124)
Browse files Browse the repository at this point in the history
* Fixed bug with slot props variables

* dont add hoisted items to context

* alternative fix for sveltejs#2586

* update slots more conservatively
  • Loading branch information
Rich-Harris authored Jun 29, 2019
1 parent 6af23ba commit b2d9da3
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 15 deletions.
6 changes: 2 additions & 4 deletions src/compiler/compile/nodes/shared/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
});
});

Expand Down
29 changes: 23 additions & 6 deletions src/compiler/compile/render_dom/wrappers/Slot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)});
Expand Down Expand Up @@ -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}) {
Expand Down
10 changes: 10 additions & 0 deletions src/compiler/compile/render_dom/wrappers/shared/is_dynamic.ts
Original file line number Diff line number Diff line change
@@ -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;
}
7 changes: 7 additions & 0 deletions test/runtime/samples/component-slot-let-e/Nested.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let fooText = 'foo';
</script>

<div>
<slot someText={fooText}></slot>
</div>
20 changes: 20 additions & 0 deletions test/runtime/samples/component-slot-let-e/_config.js
Original file line number Diff line number Diff line change
@@ -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>
`);
}
};
7 changes: 7 additions & 0 deletions test/runtime/samples/component-slot-let-e/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
import Nested from './Nested.svelte';
</script>

<Nested let:someText={someText}>
<p>{someText}</p>
</Nested>

0 comments on commit b2d9da3

Please sign in to comment.