From 75ead94ed5ac84f9ef508896acc25d19bc55b62f Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Wed, 20 May 2020 11:10:22 +0300
Subject: [PATCH] refactor(elements): simplify accessing `NgElementStrategy`
 (#36114) (#37226)

Previously, we had to check whether `NgElementStrategy` had been
instantiated before accessing it. This was tedious and error prone,
since it was easy to forget to add the check in new call sites.

This commit switches to using a getter, so that the check has to be
performed in one place and is transparent to call sites (including any
future ones).

PR Close #36114

PR Close #37226
---
 .../elements/src/create-custom-element.ts     | 51 +++++++++----------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/packages/elements/src/create-custom-element.ts b/packages/elements/src/create-custom-element.ts
index 93d5dabc2b..09652d8946 100644
--- a/packages/elements/src/create-custom-element.ts
+++ b/packages/elements/src/create-custom-element.ts
@@ -136,31 +136,35 @@ export function createCustomElement<P>(
     // field externs. So using quoted access to explicitly prevent renaming.
     static readonly['observedAttributes'] = Object.keys(attributeToPropertyInputs);
 
-    constructor(injector?: Injector) {
-      super();
-
-      // Note that some polyfills (e.g. document-register-element) do not call the constructor.
-      // Do not assume this strategy has been created.
+    protected get ngElementStrategy(): NgElementStrategy {
+      // NOTE:
+      // Some polyfills (e.g. `document-register-element`) do not call the constructor, therefore
+      // it is not safe to set `ngElementStrategy` in the constructor and assume it will be
+      // available inside the methods.
+      //
       // TODO(andrewseguin): Add e2e tests that cover cases where the constructor isn't called. For
       // now this is tested using a Google internal test suite.
-      this.ngElementStrategy = strategyFactory.create(injector || config.injector);
+      if (this._ngElementStrategy === null) {
+        this._ngElementStrategy = strategyFactory.create(this.injector);
+      }
+      return this._ngElementStrategy;
+    }
+
+    private readonly injector: Injector;
+    private _ngElementStrategy: NgElementStrategy|null = null;
+
+    constructor(injector?: Injector) {
+      super();
+      this.injector = injector || config.injector;
     }
 
     attributeChangedCallback(
         attrName: string, oldValue: string|null, newValue: string, namespace?: string): void {
-      if (!this.ngElementStrategy) {
-        this.ngElementStrategy = strategyFactory.create(config.injector);
-      }
-
       const propName = attributeToPropertyInputs[attrName]!;
       this.ngElementStrategy.setInputValue(propName, newValue);
     }
 
     connectedCallback(): void {
-      if (!this.ngElementStrategy) {
-        this.ngElementStrategy = strategyFactory.create(config.injector);
-      }
-
       this.ngElementStrategy.connect(this);
 
       // Listen for events from the strategy and dispatch them as custom events
@@ -171,8 +175,9 @@ export function createCustomElement<P>(
     }
 
     disconnectedCallback(): void {
-      if (this.ngElementStrategy) {
-        this.ngElementStrategy.disconnect();
+      // Not using `this.ngElementStrategy` to avoid unnecessarily creating the `NgElementStrategy`.
+      if (this._ngElementStrategy) {
+        this._ngElementStrategy.disconnect();
       }
 
       if (this.ngElementEventsSubscription) {
@@ -186,18 +191,8 @@ export function createCustomElement<P>(
   // contain property inputs, use all inputs by default.
   inputs.map(({propName}) => propName).forEach(property => {
     Object.defineProperty(NgElementImpl.prototype, property, {
-      get: function() {
-        if (!this.ngElementStrategy) {
-          this.ngElementStrategy = strategyFactory.create(config.injector);
-        }
-        return this.ngElementStrategy.getInputValue(property);
-      },
-      set: function(newValue: any) {
-        if (!this.ngElementStrategy) {
-          this.ngElementStrategy = strategyFactory.create(config.injector);
-        }
-        this.ngElementStrategy.setInputValue(property, newValue);
-      },
+      get: function() { return this.ngElementStrategy.getInputValue(property); },
+      set: function(newValue: any) { this.ngElementStrategy.setInputValue(property, newValue); },
       configurable: true,
       enumerable: true,
     });