From ded9aeb44710dd6cf5fec25c1ba727ec5a768ca1 Mon Sep 17 00:00:00 2001
From: Keen Yee Liau <kyliau@umich.edu>
Date: Fri, 25 Sep 2020 15:34:03 -0700
Subject: [PATCH] refactor(language-service): Move two-way binding logic to
 visitBoundEvent (#38985)

Instead of doing all sorts of checks in the `visit()` method, move checks
that are specific to `BoundEvent` to the `visitBoundEvent()` method.

PR Close #38985
---
 .../language-service/ivy/hybrid_visitor.ts    | 24 ++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/packages/language-service/ivy/hybrid_visitor.ts b/packages/language-service/ivy/hybrid_visitor.ts
index 1dec2486ad..2cc7933d92 100644
--- a/packages/language-service/ivy/hybrid_visitor.ts
+++ b/packages/language-service/ivy/hybrid_visitor.ts
@@ -58,15 +58,6 @@ class R3Visitor implements t.Visitor {
           return;
         }
       }
-      if (node instanceof t.BoundEvent &&
-          this.path.find((n => n instanceof t.BoundAttribute && node.name === n.name + 'Change'))) {
-        // For two-way binding aka banana-in-a-box, there are two matches:
-        // BoundAttribute and BoundEvent. Both have the same spans. We choose to
-        // return BoundAttribute because it matches the identifier name verbatim.
-        // TODO: For operations like go to definition, ideally we want to return
-        // both.
-        return;
-      }
       this.path.push(node);
       node.visit(this);
     }
@@ -111,9 +102,20 @@ class R3Visitor implements t.Visitor {
     visitor.visit(attribute.value, this.path);
   }
 
-  visitBoundEvent(attribute: t.BoundEvent) {
+  visitBoundEvent(event: t.BoundEvent) {
+    const isTwoWayBinding =
+        this.path.some(n => n instanceof t.BoundAttribute && event.name === n.name + 'Change');
+    if (isTwoWayBinding) {
+      // For two-way binding aka banana-in-a-box, there are two matches:
+      // BoundAttribute and BoundEvent. Both have the same spans. We choose to
+      // return BoundAttribute because it matches the identifier name verbatim.
+      // TODO: For operations like go to definition, ideally we want to return
+      // both.
+      this.path.pop();  // remove bound event from the AST path
+      return;
+    }
     const visitor = new ExpressionVisitor(this.position);
-    visitor.visit(attribute.handler, this.path);
+    visitor.visit(event.handler, this.path);
   }
 
   visitText(text: t.Text) {