From 1adbf8b29e4a856b929711730d3a0fa1b42f64a4 Mon Sep 17 00:00:00 2001 From: Chris Reeves Date: Tue, 8 Jan 2019 23:51:45 -0500 Subject: [PATCH] pull contextual deps through for member exprs in this bindings - fixes #1916 --- .../render-dom/wrappers/Element/Binding.ts | 8 ++- .../render-dom/wrappers/Element/index.ts | 21 +++++-- .../binding-this-with-context/_config.js | 59 +++++++++++++++++++ .../binding-this-with-context/main.html | 28 +++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 test/runtime/samples/binding-this-with-context/_config.js create mode 100644 test/runtime/samples/binding-this-with-context/main.html diff --git a/src/compile/render-dom/wrappers/Element/Binding.ts b/src/compile/render-dom/wrappers/Element/Binding.ts index 3877d6fa04..8b34682f8c 100644 --- a/src/compile/render-dom/wrappers/Element/Binding.ts +++ b/src/compile/render-dom/wrappers/Element/Binding.ts @@ -24,7 +24,8 @@ export default class BindingWrapper { handler: { usesContext: boolean; mutation: string; - contextual_dependencies: Set + contextual_dependencies: Set, + snippet?: string }; snippet: string; initialUpdate: string; @@ -238,9 +239,10 @@ function getEventHandler( if (binding.node.expression.node.type === 'MemberExpression') { return { - usesContext: false, + usesContext: binding.node.expression.usesContext, mutation: `${snippet} = ${value};`, - contextual_dependencies: new Set() + contextual_dependencies: binding.node.expression.contextual_dependencies, + snippet }; } diff --git a/src/compile/render-dom/wrappers/Element/index.ts b/src/compile/render-dom/wrappers/Element/index.ts index abfef21bcc..f9e89cb142 100644 --- a/src/compile/render-dom/wrappers/Element/index.ts +++ b/src/compile/render-dom/wrappers/Element/index.ts @@ -493,15 +493,28 @@ export default class ElementWrapper extends Wrapper { const { handler, object } = this_binding; + const args = []; + for (const arg of handler.contextual_dependencies) { + args.push(arg); + block.addVariable(arg, `ctx.${arg}`); + } + renderer.component.partly_hoisted.push(deindent` - function ${name}($$node) { - ${handler.mutation} + function ${name}(${['$$node', 'check'].concat(args).join(', ')}) { + ${handler.snippet ? `if ($$node || (!$$node && ${handler.snippet} === check)) ` : ''}${handler.mutation} $$invalidate('${object}', ${object}); } `); - block.builders.mount.addLine(`@add_binding_callback(() => ctx.${name}(${this.var}));`); - block.builders.destroy.addLine(`ctx.${name}(null);`); + block.builders.mount.addLine(`@add_binding_callback(() => ctx.${name}(${[this.var, 'null'].concat(args).join(', ')}));`); + block.builders.destroy.addLine(`ctx.${name}(${['null', this.var].concat(args).join(', ')});`); + block.builders.update.addLine(deindent` + if (changed.items) { + ctx.${name}(${['null', this.var].concat(args).join(', ')}); + ${args.map(a => `${a} = ctx.${a}`).join(', ')}; + ctx.${name}(${[this.var, 'null'].concat(args).join(', ')}); + }` + ); } } diff --git a/test/runtime/samples/binding-this-with-context/_config.js b/test/runtime/samples/binding-this-with-context/_config.js new file mode 100644 index 0000000000..33ef091b33 --- /dev/null +++ b/test/runtime/samples/binding-this-with-context/_config.js @@ -0,0 +1,59 @@ +export default { + html: `
foo
bar
baz
+ foobarbaz +

  • foo

  • bar

  • baz

+
`, + + test({ assert, component, target }) { + let elems = target.querySelectorAll('div'); + assert.equal(component.divs.length, 3, 'three divs are registered (unkeyed array)'); + component.divs.forEach((e, i) => { + assert.equal(e, elems[i], `div ${i} is correct (unkeyed array)`); + }); + + elems = target.querySelectorAll('span'); + assert.equal(Object.keys(component.spans).length, 3, 'three spans are registered (unkeyed object)'); + component.items.forEach((e, i) => { + assert.equal(component.spans[`-${e}${i}`], elems[i], `span -${e}${i} is correct (unkeyed object)`); + }); + + elems = target.querySelectorAll('p'); + assert.equal(component.ps.length, 3, 'three ps are registered (keyed array)'); + component.ps.forEach((e, i) => { + assert.equal(e, elems[i], `p ${i} is correct (keyed array)`); + }); + + elems = target.querySelectorAll('hr'); + assert.equal(Object.keys(component.hrs).length, 3, 'three hrs are registered (keyed object)'); + component.items.forEach((e, i) => { + assert.equal(component.hrs[e], elems[i], `hr ${e} is correct (keyed object)`); + }); + + component.items = ['foo', 'baz']; + assert.equal(component.divs.length, 3, 'the divs array is still 3 long'); + assert.equal(component.divs[2], null, 'the last div is unregistered'); + assert.equal(component.ps[2], null, 'the last p is unregistered'); + assert.equal(component.spans['-bar1'], null, 'the bar span is unregisterd'); + assert.equal(component.hrs.bar, null, 'the bar hr is unregisterd'); + + elems = target.querySelectorAll('div'); + component.divs.forEach((e, i) => { + assert.equal(e, elems[i], `div ${i} is still correct`); + }); + + elems = target.querySelectorAll('span'); + component.items.forEach((e, i) => { + assert.equal(component.spans[`-${e}${i}`], elems[i], `span -${e}${i} is still correct`); + }); + + elems = target.querySelectorAll('p'); + component.ps.forEach((e, i) => { + assert.equal(e, elems[i], `p ${i} is still correct`); + }); + + elems = target.querySelectorAll('hr'); + component.items.forEach((e, i) => { + assert.equal(component.hrs[e], elems[i], `hr ${e} is still correct`); + }); + } +}; diff --git a/test/runtime/samples/binding-this-with-context/main.html b/test/runtime/samples/binding-this-with-context/main.html new file mode 100644 index 0000000000..673800188c --- /dev/null +++ b/test/runtime/samples/binding-this-with-context/main.html @@ -0,0 +1,28 @@ + + +{#each items as item, j} +
{item}
+{/each} + +{#each Object.entries(items) as [ key, val ] } + {val} +{/each} + +
    + {#each items as thing, j (thing)} +

  • {thing}

    • + {/each} +
+ +
    + {#each items as sure, j (sure)} +
    • + {/each} +
\ No newline at end of file