refactor(@angular/cli): create a memoize decorator

With this change we clean up repeated caching code by creating a `memoize` decorator that can be used on get accessors and methods.

Author: alan-agius4

packages/angular/cli/src/command-builder/architect-command-module.ts

@@ -8,6 +8,7 @@
 
 import { Argv } from 'yargs';
 import { getProjectByCwd } from '../utilities/config';
+import { memoize } from '../utilities/memoize';
 import { ArchitectBaseCommandModule } from './architect-base-command-module';
 import {
   CommandModuleError,
@@ -110,6 +111,7 @@ export abstract class ArchitectCommandModule
     return this.commandName;
   }
 
+  @memoize
   private getProjectNamesByTarget(target: string): string[] | undefined {
     const workspace = this.getWorkspaceOrThrow();
 

packages/angular/cli/src/command-builder/command-module.ts

@@ -20,6 +20,7 @@ import {
 import { Parser as yargsParser } from 'yargs/helpers';
 import { createAnalytics } from '../analytics/analytics';
 import { AngularWorkspace } from '../utilities/config';
+import { memoize } from '../utilities/memoize';
 import { PackageManagerUtils } from '../utilities/package-manager';
 import { Option } from './utilities/json-schema';
 
@@ -169,17 +170,13 @@ export abstract class CommandModule<T extends {} = {}> implements CommandModuleI
     });
   }
 
-  private _analytics: analytics.Analytics | undefined;
-  protected async getAnalytics(): Promise<analytics.Analytics> {
-    if (this._analytics) {
-      return this._analytics;
-    }
-
-    return (this._analytics = await createAnalytics(
+  @memoize
+  protected getAnalytics(): Promise<analytics.Analytics> {
+    return createAnalytics(
       !!this.context.workspace,
       // Don't prompt for `ng update` and `ng analytics` commands.
       ['update', 'analytics'].includes(this.commandName),
-    ));
+    );
   }
 
   /**

packages/angular/cli/src/command-builder/schematics-command-module.ts

@@ -16,6 +16,7 @@ import {
 import type { CheckboxQuestion, Question } from 'inquirer';
 import { Argv } from 'yargs';
 import { getProjectByCwd, getSchematicDefaults } from '../utilities/config';
+import { memoize } from '../utilities/memoize';
 import { isTTY } from '../utilities/tty';
 import {
   CommandModule,
@@ -90,32 +91,19 @@ export abstract class SchematicsCommandModule
     return parseJsonSchemaToOptions(workflow.registry, schemaJson);
   }
 
-  private _workflowForBuilder = new Map<string, NodeWorkflow>();
+  @memoize
   protected getOrCreateWorkflowForBuilder(collectionName: string): NodeWorkflow {
-    const cached = this._workflowForBuilder.get(collectionName);
-    if (cached) {
-      return cached;
-    }
-
-    const workflow = new NodeWorkflow(this.context.root, {
+    return new NodeWorkflow(this.context.root, {
       resolvePaths: this.getResolvePaths(collectionName),
       engineHostCreator: (options) => new SchematicEngineHost(options.resolvePaths),
     });
-
-    this._workflowForBuilder.set(collectionName, workflow);
-
-    return workflow;
   }
 
-  private _workflowForExecution: NodeWorkflow | undefined;
+  @memoize
   protected async getOrCreateWorkflowForExecution(
     collectionName: string,
     options: SchematicsExecutionOptions,
   ): Promise<NodeWorkflow> {
-    if (this._workflowForExecution) {
-      return this._workflowForExecution;
-    }
-
     const { logger, root, packageManager } = this.context;
     const { force, dryRun, packageRegistry } = options;
 
@@ -241,15 +229,11 @@ export abstract class SchematicsCommandModule
       });
     }
 
-    return (this._workflowForExecution = workflow);
+    return workflow;
   }
 
-  private _schematicCollections: Set<string> | undefined;
+  @memoize
   protected async getSchematicCollections(): Promise<Set<string>> {
-    if (this._schematicCollections) {
-      return this._schematicCollections;
-    }
-
     const getSchematicCollections = (
       configSection: Record<string, unknown> | undefined,
     ): Set<string> | undefined => {
@@ -273,8 +257,6 @@ export abstract class SchematicsCommandModule
       if (project) {
         const value = getSchematicCollections(workspace.getProjectCli(project));
         if (value) {
-          this._schematicCollections = value;
-
           return value;
         }
       }
@@ -284,14 +266,10 @@ export abstract class SchematicsCommandModule
       getSchematicCollections(workspace?.getCli()) ??
       getSchematicCollections(globalConfiguration?.getCli());
     if (value) {
-      this._schematicCollections = value;
-
       return value;
     }
 
-    this._schematicCollections = new Set([DEFAULT_SCHEMATICS_COLLECTION]);
-
-    return this._schematicCollections;
+    return new Set([DEFAULT_SCHEMATICS_COLLECTION]);
   }
 
   protected parseSchematicInfo(

packages/angular/cli/src/commands/new/cli.ts

@@ -63,10 +63,14 @@ export class NewCommandModule
   async run(options: Options<NewCommandArgs> & OtherOptions): Promise<number | void> {
     // Register the version of the CLI in the registry.
     const collectionName = options.collection ?? (await this.getCollectionFromConfig());
-    const workflow = await this.getOrCreateWorkflowForExecution(collectionName, options);
-    workflow.registry.addSmartDefaultProvider('ng-cli-version', () => VERSION.full);
-
     const { dryRun, force, interactive, defaults, collection, ...schematicOptions } = options;
+    const workflow = await this.getOrCreateWorkflowForExecution(collectionName, {
+      dryRun,
+      force,
+      interactive,
+      defaults,
+    });
+    workflow.registry.addSmartDefaultProvider('ng-cli-version', () => VERSION.full);
 
     // Compatibility check for NPM 7
     if (

packages/angular/cli/src/utilities/memoize.ts

@@ -0,0 +1,84 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+/**
+ * A decorator that memoizes methods and getters.
+ *
+ * **Note**: Be cautious where and how to use this decorator as the size of the cache will grow unbounded.
+ *
+ * @see https://en.wikipedia.org/wiki/Memoization
+ */
+export function memoize<T>(
+  target: Object,
+  propertyKey: string | symbol,
+  descriptor: TypedPropertyDescriptor<T>,
+): TypedPropertyDescriptor<T> {
+  const descriptorPropertyName = descriptor.get ? 'get' : 'value';
+  const originalMethod: unknown = descriptor[descriptorPropertyName];
+
+  if (typeof originalMethod !== 'function') {
+    throw new Error('Memoize decorator can only be used on methods or get accessors.');
+  }
+
+  const cache = new Map<string, unknown>();
+
+  return {
+    ...descriptor,
+    [descriptorPropertyName]: function (this: unknown, ...args: unknown[]) {
+      for (const arg of args) {
+        if (!isJSONSerializable(arg)) {
+          throw new Error(
+            `Argument ${isNonPrimitive(arg) ? arg.toString() : arg} is JSON serializable.`,
+          );
+        }
+      }
+
+      const key = JSON.stringify(args);
+      if (cache.has(key)) {
+        return cache.get(key);
+      }
+
+      const result = originalMethod.apply(this, args);
+      cache.set(key, result);
+
+      return result;
+    },
+  };
+}
+
+/** Method to check if value is a non primitive. */
+function isNonPrimitive(value: unknown): value is object | Function | symbol {
+  return (
+    (value !== null && typeof value === 'object') ||
+    typeof value === 'function' ||
+    typeof value === 'symbol'
+  );
+}
+
+/** Method to check if the values are JSON serializable */
+function isJSONSerializable(value: unknown): boolean {
+  if (!isNonPrimitive(value)) {
+    // Can be seralized since it's a primitive.
+    return true;
+  }
+
+  let nestedValues: unknown[] | undefined;
+  if (Array.isArray(value)) {
+    // It's an array, check each item.
+    nestedValues = value;
+  } else if (Object.prototype.toString.call(value) === '[object Object]') {
+    // It's a plain object, check each value.
+    nestedValues = Object.values(value);
+  }
+
+  if (!nestedValues || nestedValues.some((v) => !isJSONSerializable(v))) {
+    return false;
+  }
+
+  return true;
+}

packages/angular/cli/src/utilities/memoize_spec.ts

@@ -0,0 +1,160 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import { memoize } from './memoize';
+
+describe('memoize', () => {
+  class Dummy {
+    @memoize
+    get random(): number {
+      return Math.random();
+    }
+
+    @memoize
+    getRandom(_parameter?: unknown): number {
+      return Math.random();
+    }
+
+    @memoize
+    async getRandomAsync(): Promise<number> {
+      return Math.random();
+    }
+  }
+
+  it('should call method once', () => {
+    const dummy = new Dummy();
+    const val1 = dummy.getRandom();
+    const val2 = dummy.getRandom();
+
+    // Should return same value since memoized
+    expect(val1).toBe(val2);
+  });
+
+  it('should call method once (async)', async () => {
+    const dummy = new Dummy();
+    const [val1, val2] = await Promise.all([dummy.getRandomAsync(), dummy.getRandomAsync()]);
+
+    // Should return same value since memoized
+    expect(val1).toBe(val2);
+  });
+
+  it('should call getter once', () => {
+    const dummy = new Dummy();
+    const val1 = dummy.random;
+    const val2 = dummy.random;
+
+    // Should return same value since memoized
+    expect(val2).toBe(val1);
+  });
+
+  it('should call method when parameter changes', () => {
+    const dummy = new Dummy();
+    const val1 = dummy.getRandom(1);
+    const val2 = dummy.getRandom(2);
+    const val3 = dummy.getRandom(1);
+    const val4 = dummy.getRandom(2);
+
+    // Should return same value since memoized
+    expect(val1).not.toBe(val2);
+    expect(val1).toBe(val3);
+    expect(val2).toBe(val4);
+  });
+
+  it('should error when used on non getters and methods', () => {
+    const test = () => {
+      class DummyError {
+        @memoize
+        set random(_value: number) {}
+      }
+
+      return new DummyError();
+    };
+
+    expect(test).toThrowError('Memoize decorator can only be used on methods or get accessors.');
+  });
+
+  describe('validate method arguments', () => {
+    it('should error when using Map', () => {
+      const test = () => new Dummy().getRandom(new Map());
+
+      expect(test).toThrowError(/Argument \[object Map\] is JSON serializable./);
+    });
+
+    it('should error when using Symbol', () => {
+      const test = () => new Dummy().getRandom(Symbol(''));
+
+      expect(test).toThrowError(/Argument Symbol\(\) is JSON serializable/);
+    });
+
+    it('should error when using Function', () => {
+      const test = () => new Dummy().getRandom(function () {});
+
+      expect(test).toThrowError(/Argument function \(\) { } is JSON serializable/);
+    });
+
+    it('should error when using Map in an array', () => {
+      const test = () => new Dummy().getRandom([new Map(), true]);
+
+      expect(test).toThrowError(/Argument \[object Map\],true is JSON serializable/);
+    });
+
+    it('should error when using Map in an Object', () => {
+      const test = () => new Dummy().getRandom({ foo: true, prop: new Map() });
+
+      expect(test).toThrowError(/Argument \[object Object\] is JSON serializable/);
+    });
+
+    it('should error when using Function in an Object', () => {
+      const test = () => new Dummy().getRandom({ foo: true, prop: function () {} });
+
+      expect(test).toThrowError(/Argument \[object Object\] is JSON serializable/);
+    });
+
+    it('should not error when using primitive values in an array', () => {
+      const test = () => new Dummy().getRandom([1, true, ['foo']]);
+
+      expect(test).not.toThrow();
+    });
+
+    it('should not error when using primitive values in an Object', () => {
+      const test = () => new Dummy().getRandom({ foo: true, prop: [1, true] });
+
+      expect(test).not.toThrow();
+    });
+
+    it('should not error when using Boolean', () => {
+      const test = () => new Dummy().getRandom(true);
+
+      expect(test).not.toThrow();
+    });
+
+    it('should not error when using String', () => {
+      const test = () => new Dummy().getRandom('foo');
+
+      expect(test).not.toThrow();
+    });
+
+    it('should not error when using Number', () => {
+      const test = () => new Dummy().getRandom(1);
+
+      expect(test).not.toThrow();
+    });
+
+    it('should not error when using null', () => {
+      const test = () => new Dummy().getRandom(null);
+
+      expect(test).not.toThrow();
+    });
+
+    it('should not error when using undefined', () => {
+      const test = () => new Dummy().getRandom(undefined);
+
+      expect(test).not.toThrow();
+    });
+  });
+});