From bea85ffe9c41e589d2a3602f28ccc0a4df71e16f Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Thu, 4 Apr 2019 13:19:38 -0700
Subject: [PATCH] fix(ivy): match microsyntax template directives correctly
 (#29698)

Previously, Template.templateAttrs was introduced to capture attribute
bindings which originated from microsyntax (e.g. bindings in *ngFor="...").
This means that a Template node can have two different structures, depending
on whether it originated from microsyntax or from a literal <ng-template>.

In the literal case, the node behaves much like an Element node, it has
attributes, inputs, and outputs which determine which directives apply.
In the microsyntax case, though, only the templateAttrs should be used
to determine which directives apply.

Previously, both the t2_binder and the TemplateDefinitionBuilder were using
the wrong set of attributes to match directives - combining the attributes,
inputs, outputs, and templateAttrs of the Template node regardless of its
origin. In the TDB's case this wasn't a problem, since the TDB collects a
global Set of directives used in the template, so it didn't matter whether
the directive was also recognized on the <ng-template>. t2_binder's API
distinguishes between directives on specific nodes, though, so it's more
sensitive to mismatching.

In particular, this showed up as an assertion failure in template type-
checking in certain cases, when a directive was accidentally matched on
a microsyntax template element and also had a binding which referenced a
variable declared in the microsyntax. This resulted in the type-checker
attempting to generate a reference to a variable that didn't exist in that
scope.

The fix is to distinguish between the two cases and select the appropriate
set of attributes to match on accordingly.

Testing strategy: tested in the t2_binder tests.

PR Close #29698
---
 packages/compiler-cli/ngcc/src/utils.ts       |  2 +-
 packages/compiler/src/render3/view/util.ts    | 18 ++++++++-------
 .../test/render3/view/binding_spec.ts         | 22 +++++++++++++++++++
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/packages/compiler-cli/ngcc/src/utils.ts b/packages/compiler-cli/ngcc/src/utils.ts
index 9146efd76a..cce404ad3d 100644
--- a/packages/compiler-cli/ngcc/src/utils.ts
+++ b/packages/compiler-cli/ngcc/src/utils.ts
@@ -48,6 +48,6 @@ export function findAll<T>(node: ts.Node, test: (node: ts.Node) => node is ts.No
  */
 export function hasNameIdentifier(declaration: ts.Declaration): declaration is ts.Declaration&
     {name: ts.Identifier} {
-  const namedDeclaration: ts.Declaration & {name?: ts.Node} = declaration;
+  const namedDeclaration: ts.Declaration&{name?: ts.Node} = declaration;
   return namedDeclaration.name !== undefined && ts.isIdentifier(namedDeclaration.name);
 }
diff --git a/packages/compiler/src/render3/view/util.ts b/packages/compiler/src/render3/view/util.ts
index 876284e988..80e5d959bc 100644
--- a/packages/compiler/src/render3/view/util.ts
+++ b/packages/compiler/src/render3/view/util.ts
@@ -165,16 +165,18 @@ export function getAttrsForDirectiveMatching(elOrTpl: t.Element | t.Template):
     {[name: string]: string} {
   const attributesMap: {[name: string]: string} = {};
 
-  elOrTpl.attributes.forEach(a => {
-    if (!isI18nAttribute(a.name)) {
-      attributesMap[a.name] = a.value;
-    }
-  });
-  elOrTpl.inputs.forEach(i => { attributesMap[i.name] = ''; });
-  elOrTpl.outputs.forEach(o => { attributesMap[o.name] = ''; });
 
-  if (elOrTpl instanceof t.Template) {
+  if (elOrTpl instanceof t.Template && elOrTpl.tagName !== 'ng-template') {
     elOrTpl.templateAttrs.forEach(a => attributesMap[a.name] = '');
+  } else {
+    elOrTpl.attributes.forEach(a => {
+      if (!isI18nAttribute(a.name)) {
+        attributesMap[a.name] = a.value;
+      }
+    });
+
+    elOrTpl.inputs.forEach(i => { attributesMap[i.name] = ''; });
+    elOrTpl.outputs.forEach(o => { attributesMap[o.name] = ''; });
   }
 
   return attributesMap;
diff --git a/packages/compiler/test/render3/view/binding_spec.ts b/packages/compiler/test/render3/view/binding_spec.ts
index 85ca63f4ba..946bac59df 100644
--- a/packages/compiler/test/render3/view/binding_spec.ts
+++ b/packages/compiler/test/render3/view/binding_spec.ts
@@ -24,6 +24,13 @@ function makeSelectorMatcher(): SelectorMatcher<DirectiveMeta> {
     outputs: {},
     isComponent: false,
   });
+  matcher.addSelectables(CssSelector.parse('[dir]'), {
+    name: 'Dir',
+    exportAs: null,
+    inputs: {},
+    outputs: {},
+    isComponent: false,
+  });
   return matcher;
 }
 
@@ -56,4 +63,19 @@ describe('t2 binding', () => {
     expect(directives.length).toBe(1);
     expect(directives[0].name).toBe('NgFor');
   });
+
+  it('should not match directives intended for an element on a microsyntax template', () => {
+    const template = parseTemplate('<div *ngFor="let item of items" dir></div>', '', {});
+    const binder = new R3TargetBinder(makeSelectorMatcher());
+    const res = binder.bind({template: template.nodes});
+    const tmpl = template.nodes[0] as a.Template;
+    const tmplDirectives = res.getDirectivesOfNode(tmpl) !;
+    expect(tmplDirectives).not.toBeNull();
+    expect(tmplDirectives.length).toBe(1);
+    expect(tmplDirectives[0].name).toBe('NgFor');
+    const elDirectives = res.getDirectivesOfNode(tmpl.children[0] as a.Element) !;
+    expect(elDirectives).not.toBeNull();
+    expect(elDirectives.length).toBe(1);
+    expect(elDirectives[0].name).toBe('Dir');
+  });
 });
\ No newline at end of file