fix some slot bugs

pull/1998/head
Richard Harris 7 years ago
parent a9ff0ee022
commit 8b0dd112b9

@ -12,7 +12,7 @@ export default class CatchBlock extends Node {
super(component, parent, scope, info); super(component, parent, scope, info);
this.scope = scope.child(); this.scope = scope.child();
this.scope.add(parent.error, parent.expression.dependencies); this.scope.add(parent.error, parent.expression.dependencies, this);
this.children = mapChildren(component, parent, this.scope, info.children); this.children = mapChildren(component, parent, this.scope, info.children);
this.warnIfEmptyBlock(); this.warnIfEmptyBlock();

@ -20,6 +20,7 @@ export default class EachBlock extends Node {
scope: TemplateScope; scope: TemplateScope;
contexts: Array<{ name: string, tail: string }>; contexts: Array<{ name: string, tail: string }>;
hasAnimation: boolean; hasAnimation: boolean;
has_binding = false;
children: Node[]; children: Node[];
else?: ElseBlock; else?: ElseBlock;
@ -38,7 +39,7 @@ export default class EachBlock extends Node {
unpackDestructuring(this.contexts, info.context, ''); unpackDestructuring(this.contexts, info.context, '');
this.contexts.forEach(context => { this.contexts.forEach(context => {
this.scope.add(context.key.name, this.expression.dependencies); this.scope.add(context.key.name, this.expression.dependencies, this);
}); });
this.key = info.key this.key = info.key
@ -48,7 +49,7 @@ export default class EachBlock extends Node {
if (this.index) { if (this.index) {
// index can only change if this is a keyed each block // index can only change if this is a keyed each block
const dependencies = this.key ? this.expression.dependencies : []; const dependencies = this.key ? this.expression.dependencies : [];
this.scope.add(this.index, dependencies); this.scope.add(this.index, dependencies, this);
} }
this.hasAnimation = false; this.hasAnimation = false;

@ -82,7 +82,7 @@ export default class InlineComponent extends Node {
const dependencies = new Set([l.name]); const dependencies = new Set([l.name]);
l.names.forEach(name => { l.names.forEach(name => {
this.scope.add(name, dependencies); this.scope.add(name, dependencies, this);
}); });
}); });
} else { } else {

@ -12,7 +12,7 @@ export default class ThenBlock extends Node {
super(component, parent, scope, info); super(component, parent, scope, info);
this.scope = scope.child(); this.scope = scope.child();
this.scope.add(parent.value, parent.expression.dependencies); this.scope.add(parent.value, parent.expression.dependencies, this);
this.children = mapChildren(component, parent, this.scope, info.children); this.children = mapChildren(component, parent, this.scope, info.children);
this.warnIfEmptyBlock(); this.warnIfEmptyBlock();

@ -113,7 +113,10 @@ export default class Expression {
// function, and it only applies if the dependency is writable // function, and it only applies if the dependency is writable
// or a sub-path of a non-writable // or a sub-path of a non-writable
if (component.instance_script) { if (component.instance_script) {
if (component.writable_declarations.has(name) || name[0] === '$' || (component.userVars.has(name) && deep)) { const owner = template_scope.getOwner(name);
const is_let = owner && (owner.type === 'InlineComponent' || owner.type === 'Element');
if (is_let || component.writable_declarations.has(name) || name[0] === '$' || (component.userVars.has(name) && deep)) {
dynamic_dependencies.add(name); dynamic_dependencies.add(name);
} }
} else { } else {

@ -1,19 +1,28 @@
import Node from './Node';
import EachBlock from '../EachBlock';
import ThenBlock from '../ThenBlock';
import CatchBlock from '../CatchBlock';
import InlineComponent from '../InlineComponent';
type NodeWithScope = EachBlock | ThenBlock | CatchBlock | InlineComponent | Element;
export default class TemplateScope { export default class TemplateScope {
names: Set<string>; names: Set<string>;
dependenciesForName: Map<string, Set<string>>; dependenciesForName: Map<string, Set<string>>;
mutables: Set<string>; mutables: Set<string> = new Set();
owners: Map<string, NodeWithScope> = new Map();
parent?: TemplateScope; parent?: TemplateScope;
constructor(parent?: TemplateScope) { constructor(parent?: TemplateScope) {
this.parent = parent; this.parent = parent;
this.names = new Set(parent ? parent.names : []); this.names = new Set(parent ? parent.names : []);
this.dependenciesForName = new Map(parent ? parent.dependenciesForName : []); this.dependenciesForName = new Map(parent ? parent.dependenciesForName : []);
this.mutables = new Set();
} }
add(name, dependencies: Set<string>) { add(name, dependencies: Set<string>, owner) {
this.names.add(name); this.names.add(name);
this.dependenciesForName.set(name, dependencies); this.dependenciesForName.set(name, dependencies);
this.owners.set(name, owner);
return this; return this;
} }
@ -32,6 +41,10 @@ export default class TemplateScope {
containsMutable(names: Iterable<string>) { containsMutable(names: Iterable<string>) {
for (const name of names) { for (const name of names) {
const owner = this.getOwner(name);
const is_let = owner && (owner.type === 'InlineComponent' || owner.type === 'Element');
if (is_let) return true;
if (name[0] === '$') return true; if (name[0] === '$') return true;
if (this.mutables.has(name)) return true; if (this.mutables.has(name)) return true;
else if (this.dependenciesForName.has(name) && this.containsMutable(this.dependenciesForName.get(name))) return true; else if (this.dependenciesForName.has(name) && this.containsMutable(this.dependenciesForName.get(name))) return true;
@ -44,4 +57,8 @@ export default class TemplateScope {
isTopLevel(name: string) { isTopLevel(name: string) {
return !this.parent || !this.names.has(name) && this.parent.isTopLevel(name); return !this.parent || !this.names.has(name) && this.parent.isTopLevel(name);
} }
getOwner(name: string): NodeWithScope {
return this.owners.get(name) || (this.parent && this.parent.getOwner(name));
}
} }

@ -1,9 +1,10 @@
import CodeBuilder from '../../utils/CodeBuilder'; import CodeBuilder from '../../utils/CodeBuilder';
import deindent from '../../utils/deindent'; import deindent from '../../utils/deindent';
import { escape } from '../../utils/stringify';
import Renderer from './Renderer'; import Renderer from './Renderer';
import Wrapper from './wrappers/shared/Wrapper'; import Wrapper from './wrappers/shared/Wrapper';
import EachBlockWrapper from './wrappers/EachBlock'; import EachBlockWrapper from './wrappers/EachBlock';
import InlineComponentWrapper from './wrappers/InlineComponent';
import ElementWrapper from './wrappers/Element';
export interface BlockOptions { export interface BlockOptions {
parent?: Block; parent?: Block;
@ -12,7 +13,6 @@ export interface BlockOptions {
comment?: string; comment?: string;
key?: string; key?: string;
bindings?: Map<string, () => { object: string, property: string, snippet: string }>; bindings?: Map<string, () => { object: string, property: string, snippet: string }>;
contextOwners?: Map<string, EachBlockWrapper>;
dependencies?: Set<string>; dependencies?: Set<string>;
} }
@ -30,7 +30,6 @@ export default class Block {
dependencies: Set<string>; dependencies: Set<string>;
bindings: Map<string, { object: string, property: string, snippet: string }>; bindings: Map<string, { object: string, property: string, snippet: string }>;
contextOwners: Map<string, EachBlockWrapper>;
builders: { builders: {
init: CodeBuilder; init: CodeBuilder;
@ -79,7 +78,6 @@ export default class Block {
this.dependencies = new Set(); this.dependencies = new Set();
this.bindings = options.bindings; this.bindings = options.bindings;
this.contextOwners = options.contextOwners;
this.builders = { this.builders = {
init: new CodeBuilder(), init: new CodeBuilder(),

@ -44,7 +44,6 @@ export default class Renderer {
key: null, key: null,
bindings: new Map(), bindings: new Map(),
contextOwners: new Map(),
dependencies: new Set(), dependencies: new Set(),
}); });

@ -62,7 +62,6 @@ export default class EachBlockWrapper extends Wrapper {
indexName: string; indexName: string;
var = 'each'; var = 'each';
hasBinding = false;
constructor( constructor(
renderer: Renderer, renderer: Renderer,
@ -83,8 +82,7 @@ export default class EachBlockWrapper extends Wrapper {
name: renderer.component.getUniqueName('create_each_block'), name: renderer.component.getUniqueName('create_each_block'),
key: <string>node.key, // TODO... key: <string>node.key, // TODO...
bindings: new Map(block.bindings), bindings: new Map(block.bindings)
contextOwners: new Map(block.contextOwners)
}); });
// TODO this seems messy // TODO this seems messy
@ -110,8 +108,6 @@ export default class EachBlockWrapper extends Wrapper {
}; };
node.contexts.forEach(prop => { node.contexts.forEach(prop => {
this.block.contextOwners.set(prop.key.name, this);
this.block.bindings.set(prop.key.name, { this.block.bindings.set(prop.key.name, {
object: this.vars.each_block_value, object: this.vars.each_block_value,
property: this.indexName, property: this.indexName,
@ -167,8 +163,8 @@ export default class EachBlockWrapper extends Wrapper {
this.contextProps = this.node.contexts.map(prop => `child_ctx.${prop.key.name} = list[i]${prop.tail};`); this.contextProps = this.node.contexts.map(prop => `child_ctx.${prop.key.name} = list[i]${prop.tail};`);
if (this.hasBinding) this.contextProps.push(`child_ctx.${this.vars.each_block_value} = list;`); if (this.node.has_binding) this.contextProps.push(`child_ctx.${this.vars.each_block_value} = list;`);
if (this.hasBinding || this.node.index) this.contextProps.push(`child_ctx.${this.indexName} = i;`); if (this.node.has_binding || this.node.index) this.contextProps.push(`child_ctx.${this.indexName} = i;`);
const snippet = this.node.expression.render(block); const snippet = this.node.expression.render(block);

@ -7,6 +7,7 @@ import Node from '../../../nodes/shared/Node';
import Renderer from '../../Renderer'; import Renderer from '../../Renderer';
import flattenReference from '../../../../utils/flattenReference'; import flattenReference from '../../../../utils/flattenReference';
import { get_tail } from '../../../../utils/get_tail_snippet'; import { get_tail } from '../../../../utils/get_tail_snippet';
import EachBlock from '../../../nodes/EachBlock';
// TODO this should live in a specific binding // TODO this should live in a specific binding
const readOnlyMediaAttributes = new Set([ const readOnlyMediaAttributes = new Set([
@ -52,9 +53,9 @@ export default class BindingWrapper {
// we need to ensure that the each block creates a context including // we need to ensure that the each block creates a context including
// the list and the index, if they're not otherwise referenced // the list and the index, if they're not otherwise referenced
const { name } = getObject(this.node.expression.node); const { name } = getObject(this.node.expression.node);
const eachBlock = block.contextOwners.get(name); const eachBlock = this.parent.node.scope.getOwner(name);
eachBlock.hasBinding = true; (<EachBlock>eachBlock).has_binding = true;
} }
this.object = getObject(this.node.expression.node).name; this.object = getObject(this.node.expression.node).name;

@ -21,6 +21,7 @@ import addEventHandlers from '../shared/addEventHandlers';
import addActions from '../shared/addActions'; import addActions from '../shared/addActions';
import createDebuggingComment from '../../../../utils/createDebuggingComment'; import createDebuggingComment from '../../../../utils/createDebuggingComment';
import sanitize from '../../../../utils/sanitize'; import sanitize from '../../../../utils/sanitize';
import { get_context_merger } from '../shared/get_context_merger';
const events = [ const events = [
{ {
@ -136,7 +137,7 @@ export default class ElementWrapper extends Wrapper {
name: this.renderer.component.getUniqueName(`create_${sanitize(name)}_slot`) name: this.renderer.component.getUniqueName(`create_${sanitize(name)}_slot`)
}); });
const fn = `({ thing }) => ({ thing })`; const fn = get_context_merger(this.node.lets);
(<InlineComponentWrapper>owner).slots.set(name, { (<InlineComponentWrapper>owner).slots.set(name, {
block: child_block, block: child_block,

@ -63,6 +63,10 @@ export default class FragmentWrapper {
while (i--) { while (i--) {
const child = nodes[i]; const child = nodes[i];
if (!child.type) {
throw new Error(`missing type`)
}
if (!(child.type in wrappers)) { if (!(child.type in wrappers)) {
throw new Error(`TODO implement ${child.type}`); throw new Error(`TODO implement ${child.type}`);
} }

@ -14,6 +14,7 @@ import flattenReference from '../../../../utils/flattenReference';
import createDebuggingComment from '../../../../utils/createDebuggingComment'; import createDebuggingComment from '../../../../utils/createDebuggingComment';
import sanitize from '../../../../utils/sanitize'; import sanitize from '../../../../utils/sanitize';
import { get_context_merger } from '../shared/get_context_merger'; import { get_context_merger } from '../shared/get_context_merger';
import EachBlock from '../../../nodes/EachBlock';
export default class InlineComponentWrapper extends Wrapper { export default class InlineComponentWrapper extends Wrapper {
var: string; var: string;
@ -46,9 +47,9 @@ export default class InlineComponentWrapper extends Wrapper {
// we need to ensure that the each block creates a context including // we need to ensure that the each block creates a context including
// the list and the index, if they're not otherwise referenced // the list and the index, if they're not otherwise referenced
const { name } = getObject(binding.expression.node); const { name } = getObject(binding.expression.node);
const eachBlock = block.contextOwners.get(name); const eachBlock = this.node.scope.getOwner(name);
eachBlock.hasBinding = true; (<EachBlock>eachBlock).has_binding = true;
} }
block.addDependencies(binding.expression.dynamic_dependencies); block.addDependencies(binding.expression.dynamic_dependencies);
@ -71,6 +72,7 @@ export default class InlineComponentWrapper extends Wrapper {
comment: createDebuggingComment(node, renderer.component), comment: createDebuggingComment(node, renderer.component),
name: renderer.component.getUniqueName(`create_default_slot`) name: renderer.component.getUniqueName(`create_default_slot`)
}); });
this.renderer.blocks.push(default_slot); this.renderer.blocks.push(default_slot);
const fn = get_context_merger(this.node.lets); const fn = get_context_merger(this.node.lets);

@ -2,16 +2,17 @@ import Wrapper from './shared/Wrapper';
import Renderer from '../Renderer'; import Renderer from '../Renderer';
import Block from '../Block'; import Block from '../Block';
import Slot from '../../nodes/Slot'; import Slot from '../../nodes/Slot';
import { quotePropIfNecessary } from '../../../utils/quoteIfNecessary';
import FragmentWrapper from './Fragment'; import FragmentWrapper from './Fragment';
import deindent from '../../../utils/deindent'; import deindent from '../../../utils/deindent';
import sanitize from '../../../utils/sanitize'; import sanitize from '../../../utils/sanitize';
import addToSet from '../../../utils/addToSet';
export default class SlotWrapper extends Wrapper { export default class SlotWrapper extends Wrapper {
node: Slot; node: Slot;
fragment: FragmentWrapper; fragment: FragmentWrapper;
var = 'slot'; var = 'slot';
dependencies: Set<string> = new Set(['$$scope']);
constructor( constructor(
renderer: Renderer, renderer: Renderer,
@ -33,7 +34,11 @@ export default class SlotWrapper extends Wrapper {
nextSibling nextSibling
); );
block.addDependencies(new Set(['$$scope'])); this.node.attributes.forEach(attribute => {
addToSet(this.dependencies, attribute.dependencies);
});
block.addDependencies(this.dependencies);
} }
render( render(
@ -90,9 +95,14 @@ export default class SlotWrapper extends Wrapper {
} }
`); `);
block.builders.update.addLine( let update_conditions = [...this.dependencies].map(name => `changed.${name}`).join(' || ');
`if (${slot} && changed.$$scope) ${slot}.p(ctx.$$scope.changed, ctx.$$scope.ctx);` if (this.dependencies.size > 1) update_conditions = `(${update_conditions})`;
);
block.builders.update.addBlock(deindent`
if (${slot} && ${update_conditions}) {
${slot}.p(@assign(@assign({}, changed), ctx.$$scope.changed), @get_slot_context(ctx.$$slot_${sanitize(slot_name)}, ctx));
}
`);
block.builders.destroy.addLine( block.builders.destroy.addLine(
`if (${slot}) ${slot}.d(detach);` `if (${slot}) ${slot}.d(detach);`

@ -49,10 +49,14 @@ export function validate_store(store, name) {
export function create_slot(definition, ctx) { export function create_slot(definition, ctx) {
if (definition) { if (definition) {
const slot_ctx = definition[1] const slot_ctx = get_slot_context(definition, ctx);
? assign({}, assign(ctx.$$scope.ctx, definition[1](ctx)))
: ctx.$$scope.ctx;
return definition[0](slot_ctx); return definition[0](slot_ctx);
} }
} }
export function get_slot_context(definition, ctx) {
return definition[1]
? assign({}, assign(ctx.$$scope.ctx, definition[1](ctx)))
: ctx.$$scope.ctx
}

Loading…
Cancel
Save