From 3e201181bb9bc8da15b2a808a34c742458149851 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 31 Jul 2019 14:47:25 -0700 Subject: [PATCH] fix(ivy): correctly associate output bound events with directives (#31938) Previously, bound events were incorrectly bound to directives with inputs matching the bound event attribute. This fixes that so bound events can only be bound to directives with matching outputs. Adds tests for all kinds of directive matching on bound attributes. PR Close #31938 --- .../compiler/src/render3/view/t2_binder.ts | 27 ++++--- .../test/render3/view/binding_spec.ts | 80 +++++++++++++++++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index 9bb9f0630a..3d677daa1a 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -269,20 +269,23 @@ class DirectiveBinder implements Visitor { }); // Associate attributes/bindings on the node with directives or with the node itself. - const processAttribute = (attribute: BoundAttribute | BoundEvent | TextAttribute) => { - let dir = directives.find(dir => dir.inputs.hasOwnProperty(attribute.name)); - if (dir !== undefined) { - this.bindings.set(attribute, dir); - } else { - this.bindings.set(attribute, node); - } - }; - node.attributes.forEach(processAttribute); - node.inputs.forEach(processAttribute); - node.outputs.forEach(processAttribute); + type BoundNode = BoundAttribute | BoundEvent | TextAttribute; + const setAttributeBinding = + (attribute: BoundNode, ioType: keyof Pick) => { + const dir = directives.find(dir => dir[ioType].hasOwnProperty(attribute.name)); + const binding = dir !== undefined ? dir : node; + this.bindings.set(attribute, binding); + }; + + // Node inputs (bound attributes) and text attributes can be bound to an + // input on a directive. + node.inputs.forEach(input => setAttributeBinding(input, 'inputs')); + node.attributes.forEach(attr => setAttributeBinding(attr, 'inputs')); if (node instanceof Template) { - node.templateAttrs.forEach(processAttribute); + node.templateAttrs.forEach(attr => setAttributeBinding(attr, 'inputs')); } + // Node outputs (bound events) can be bound to an output on a directive. + node.outputs.forEach(output => setAttributeBinding(output, 'outputs')); // Recurse into the node's children. node.children.forEach(child => child.visit(this)); diff --git a/packages/compiler/test/render3/view/binding_spec.ts b/packages/compiler/test/render3/view/binding_spec.ts index 8eb08599f1..e47c57d18b 100644 --- a/packages/compiler/test/render3/view/binding_spec.ts +++ b/packages/compiler/test/render3/view/binding_spec.ts @@ -31,6 +31,20 @@ function makeSelectorMatcher(): SelectorMatcher { outputs: {}, isComponent: false, }); + matcher.addSelectables(CssSelector.parse('[hasOutput]'), { + name: 'HasOutput', + exportAs: null, + inputs: {}, + outputs: {'outputBinding': 'outputBinding'}, + isComponent: false, + }); + matcher.addSelectables(CssSelector.parse('[hasInput]'), { + name: 'HasInput', + exportAs: null, + inputs: {'inputBinding': 'inputBinding'}, + outputs: {}, + isComponent: false, + }); return matcher; } @@ -98,4 +112,70 @@ describe('t2 binding', () => { expect(elDirectives.length).toBe(1); expect(elDirectives[0].name).toBe('Dir'); }); + + describe('matching inputs to consuming directives', () => { + it('should work for bound attributes', () => { + const template = parseTemplate('
', '', {}); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const el = template.nodes[0] as a.Element; + const attr = el.inputs[0]; + const consumer = res.getConsumerOfBinding(attr) as DirectiveMeta; + expect(consumer.name).toBe('HasInput'); + }); + + it('should work for text attributes on elements', () => { + const template = parseTemplate('
', '', {}); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const el = template.nodes[0] as a.Element; + const attr = el.attributes[1]; + const consumer = res.getConsumerOfBinding(attr) as DirectiveMeta; + expect(consumer.name).toBe('HasInput'); + }); + + it('should work for text attributes on templates', () => { + const template = + parseTemplate('', '', {}); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const el = template.nodes[0] as a.Element; + const attr = el.attributes[1]; + const consumer = res.getConsumerOfBinding(attr) as DirectiveMeta; + expect(consumer.name).toBe('HasInput'); + }); + + it('should bind to the encompassing node when no directive input is matched', () => { + const template = parseTemplate('', '', {}); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const el = template.nodes[0] as a.Element; + const attr = el.attributes[0]; + const consumer = res.getConsumerOfBinding(attr); + expect(consumer).toEqual(el); + }); + }); + + describe('matching outputs to consuming directives', () => { + it('should work for bound events', () => { + const template = + parseTemplate('
', '', {}); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const el = template.nodes[0] as a.Element; + const attr = el.outputs[0]; + const consumer = res.getConsumerOfBinding(attr) as DirectiveMeta; + expect(consumer.name).toBe('HasOutput'); + }); + + it('should bind to the encompassing node when no directive output is matched', () => { + const template = parseTemplate('', '', {}); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const el = template.nodes[0] as a.Element; + const attr = el.outputs[0]; + const consumer = res.getConsumerOfBinding(attr); + expect(consumer).toEqual(el); + }); + }); });