diff --git a/src/compile/render-dom/wrappers/InlineComponent/index.ts b/src/compile/render-dom/wrappers/InlineComponent/index.ts index 654de5cac9..216d4f3fcd 100644 --- a/src/compile/render-dom/wrappers/InlineComponent/index.ts +++ b/src/compile/render-dom/wrappers/InlineComponent/index.ts @@ -252,10 +252,47 @@ export default class InlineComponentWrapper extends Wrapper { block.maintainContext = true; // TODO put this somewhere more logical } - this.node.handlers.forEach(handler => { - handler.var = block.getUniqueName(`${this.var}_${handler.name}`); // TODO this is hacky - // handler.render(component, block, this.var, false); // TODO hoist when possible - if (handler.usesContext) block.maintainContext = true; // TODO is there a better place to put this? + const munged_handlers = this.node.handlers.map(handler => { + let { snippet } = handler; + + // get a name for the event handler that is globally unique + const handler_name = component.getUniqueName( + `${handler.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_handler` + ); + + if (handler.expression.contextual_dependencies.size > 0) { + block.maintainContext = true; // TODO is there a better place to put this? + + const deps = Array.from(handler.expression.contextual_dependencies); + + component.event_handlers.push({ + name: handler_name, + body: deindent` + function ${handler_name}(event, { ${deps.join(', ')} }) { + (${snippet})(event); + } + ` + }); + + block.builders.init.addBlock(deindent` + function ${handler.name}(event) { + ctx.${handler_name}.call(this, event, ctx); + } + `); + + return `${name}.$on("${handler.name}", ${handler_name})`; + } + + component.event_handlers.push({ + name: handler_name, + body: deindent` + function ${handler_name}(event) { + (${snippet})(event); + } + ` + }); + + return `${name}.$on("${handler.name}", ctx.${handler_name})`; }); if (this.node.name === 'svelte:component') { @@ -280,15 +317,8 @@ export default class InlineComponentWrapper extends Wrapper { var ${name} = new ${switch_value}(${switch_props}(ctx)); ${munged_bindings} + ${munged_handlers} } - - ${this.node.handlers.map(handler => deindent` - function ${handler.var}(event) { - (${handler.snippet || `() => { throw new Error('TODO shorthand events'); }`})(event); - } - - if (${name}) ${name}.$on("${handler.name}", ${handler.var}); - `)} `); block.builders.create.addLine( @@ -334,6 +364,8 @@ export default class InlineComponentWrapper extends Wrapper { ${name} = new ${switch_value}(${switch_props}(ctx)); ${munged_bindings} + ${munged_handlers} + ${name}.$$fragment.c(); ${this.fragment && this.fragment.nodes.map(child => child.remount(name))} @@ -378,50 +410,11 @@ export default class InlineComponentWrapper extends Wrapper { }); ${munged_bindings} + ${munged_handlers} ${this.node.ref && `#component.$$refs.${this.node.ref.name} = ${name};`} `); - this.node.handlers.forEach(handler => { - let { snippet } = handler; - if (!snippet) snippet = `() => { throw new Error('TODO shorthand events'); }`; - - // get a name for the event handler that is globally unique - const handler_name = component.getUniqueName( - `${handler.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_handler` - ); - - if (handler.expression.contextual_dependencies.size > 0) { - const deps = Array.from(handler.expression.contextual_dependencies); - - component.event_handlers.push({ - name: handler_name, - body: deindent` - function ${handler_name}(event, { ${deps.join(', ')} }) { - (${snippet})(event); - } - ` - }); - - block.builders.init.addBlock(deindent` - ${name}.$on("${handler.name}", function(event) { - ctx.${handler_name}.call(this, event, ctx); - }); - `); - } else { - component.event_handlers.push({ - name: handler_name, - body: deindent` - function ${handler_name}(event) { - (${snippet})(event); - } - ` - }); - - block.builders.init.addLine(`${name}.$on("${handler.name}", ctx.${handler_name});`); - } - }); - block.builders.create.addLine(`${name}.$$fragment.c();`); if (parentNodes) { diff --git a/test/runtime/samples/dynamic-component-bindings-recreated-b/_config.js b/test/runtime/samples/dynamic-component-bindings-recreated-b/_config.js index 95a64a4a41..3e1c61702b 100644 --- a/test/runtime/samples/dynamic-component-bindings-recreated-b/_config.js +++ b/test/runtime/samples/dynamic-component-bindings-recreated-b/_config.js @@ -9,20 +9,16 @@ export default { `, test(assert, component, target) { - // TODO replace this with component.set({ foo: undefined }) post-#1488 - // component.set({ foo: undefined }); - // delete component._state.foo; - - component.x = false; component.foo = undefined; + component.x = false; assert.htmlEqual(target.innerHTML, `

parent red

red red

`); - component.x = true; component.foo = undefined; + component.x = true; assert.htmlEqual(target.innerHTML, `

parent green

diff --git a/test/runtime/samples/dynamic-component-bindings-recreated/_config.js b/test/runtime/samples/dynamic-component-bindings-recreated/_config.js index 8b9811bf99..1ac057ca3e 100644 --- a/test/runtime/samples/dynamic-component-bindings-recreated/_config.js +++ b/test/runtime/samples/dynamic-component-bindings-recreated/_config.js @@ -15,8 +15,8 @@ export default {

red one

`); - component.x = true; component.foo = 'two'; + component.x = true; assert.htmlEqual(target.innerHTML, `

green two

diff --git a/test/runtime/samples/dynamic-component-events/main.html b/test/runtime/samples/dynamic-component-events/main.html index 68b9551350..77856e8e63 100644 --- a/test/runtime/samples/dynamic-component-events/main.html +++ b/test/runtime/samples/dynamic-component-events/main.html @@ -6,4 +6,4 @@ export let selected; - \ No newline at end of file + \ No newline at end of file