Add tests for @cached decorator

glimmerjs · Oct 5, 2021 · 427fe2a · 427fe2a
1 parent 3b937dd
commit 427fe2a
Show file tree

Hide file tree

Showing 5 changed files with 242 additions and 6 deletions.
diff --git a/packages/@glimmer/tracking/src/cached.ts b/packages/@glimmer/tracking/src/cached.ts
@@ -73,13 +73,18 @@ export const cached: PropertyDecorator = (...args: any[]) => {
   ) {
     throwCachedInvalidArgsError(args);
   }
-  if (DEBUG && 'get' in descriptor && typeof descriptor.get !== 'function')
+  if (DEBUG && (!('get' in descriptor) || typeof descriptor.get !== 'function')) {
     throwCachedGetterOnlyError(key);
+  }
 
   const caches = new WeakMap();
   const getter = descriptor.get;
+
   descriptor.get = function (): unknown {
-    if (!caches.has(this)) caches.set(this, createCache(getter.bind(this)));
+    if (!caches.has(this)) {
+      caches.set(this, createCache(getter.bind(this)));
+    }
+
     return getValue(caches.get(this));
   };
 };
@@ -94,7 +99,7 @@ function throwCachedGetterOnlyError(key: string): never {
   throw new Error(`The @cached decorator must be applied to getters. '${key}' is not a getter.`);
 }
 
-function throwCachedInvalidArgsError(args: unknown[]): never {
+function throwCachedInvalidArgsError(args: unknown[] = []): never {
   throw new Error(
     `You attempted to use @cached on with ${
       args.length > 1 ? 'arguments' : 'an argument'

diff --git a/packages/@glimmer/tracking/test/cached-decorator-test.ts b/packages/@glimmer/tracking/test/cached-decorator-test.ts
@@ -0,0 +1,132 @@
+/* eslint-disable @typescript-eslint/explicit-function-return-type */
+const { test } = QUnit;
+
+import { DEBUG } from '@glimmer/env';
+import { tracked, cached } from '@glimmer/tracking';
+
+import * as TSFixtures from './fixtures/typescript';
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
+// @ts-ignore
+import * as BabelFixtures from './fixtures/babel';
+
+QUnit.module('[@glimmer/tracking] @cached Decorators');
+
+test('it works', function (assert) {
+  class Person {
+    @tracked firstName = 'Jen';
+    @tracked lastName = 'Weber';
+
+    @cached
+    get fullName() {
+      const fullName = `${this.firstName} ${this.lastName}`;
+      assert.step(fullName);
+      return fullName;
+    }
+  }
+
+  const person = new Person();
+  assert.verifySteps([], 'getter is not called after class initialization');
+
+  assert.strictEqual(person.fullName, 'Jen Weber');
+  assert.verifySteps(['Jen Weber'], 'getter was called after property access');
+
+  assert.strictEqual(person.fullName, 'Jen Weber');
+  assert.verifySteps([], 'getter was not called again after repeated property access');
+
+  person.firstName = 'Kenneth';
+  assert.verifySteps([], 'changing a property does not trigger an eager re-computation');
+
+  assert.strictEqual(person.fullName, 'Kenneth Weber');
+  assert.verifySteps(['Kenneth Weber'], 'accessing the property triggers a re-computation');
+
+  assert.strictEqual(person.fullName, 'Kenneth Weber');
+  assert.verifySteps([], 'getter was not called again after repeated property access');
+
+  person.lastName = 'Larsen';
+  assert.verifySteps([], 'changing a property does not trigger an eager re-computation');
+
+  assert.strictEqual(person.fullName, 'Kenneth Larsen');
+  assert.verifySteps(['Kenneth Larsen'], 'accessing the property triggers a re-computation');
+});
+
+// https://github.com/ember-polyfills/ember-cached-decorator-polyfill/issues/7
+test('it has a separate cache per class instance', function (assert) {
+  class Person {
+    @tracked firstName: string;
+    @tracked lastName: string;
+
+    constructor(firstName: string, lastName: string) {
+      this.firstName = firstName;
+      this.lastName = lastName;
+    }
+
+    @cached
+    get fullName() {
+      const fullName = `${this.firstName} ${this.lastName}`;
+      assert.step(fullName);
+      return fullName;
+    }
+  }
+
+  const jen = new Person('Jen', 'Weber');
+  const chris = new Person('Chris', 'Garrett');
+
+  assert.verifySteps([], 'getter is not called after class initialization');
+
+  assert.strictEqual(jen.fullName, 'Jen Weber');
+  assert.verifySteps(['Jen Weber'], 'getter was called after property access');
+
+  assert.strictEqual(jen.fullName, 'Jen Weber');
+  assert.verifySteps([], 'getter was not called again after repeated property access');
+
+  assert.strictEqual(chris.fullName, 'Chris Garrett', 'other instance has a different value');
+  assert.verifySteps(['Chris Garrett'], 'getter was called after property access');
+
+  assert.strictEqual(chris.fullName, 'Chris Garrett');
+  assert.verifySteps([], 'getter was not called again after repeated property access');
+
+  chris.lastName = 'Manson';
+  assert.verifySteps([], 'changing a property does not trigger an eager re-computation');
+
+  assert.strictEqual(jen.fullName, 'Jen Weber', 'other instance is unaffected');
+  assert.verifySteps([], 'getter was not called again after repeated property access');
+
+  assert.strictEqual(chris.fullName, 'Chris Manson');
+  assert.verifySteps(['Chris Manson'], 'getter was called after property access');
+
+  assert.strictEqual(jen.fullName, 'Jen Weber', 'other instance is unaffected');
+  assert.verifySteps([], 'getter was not called again after repeated property access');
+});
+
+[
+  ['Babel', BabelFixtures],
+  ['TypeScript', TSFixtures],
+].forEach(([compiler, F]) => {
+  QUnit.module(`[@glimmer/tracking] Cached Property Decorators with ${compiler}`);
+
+  if (DEBUG) {
+    test('Cached decorator on a property throws an error', (assert) => {
+      assert.throws(F.createClassWithCachedProperty);
+    });
+
+    test('Cached decorator with a setter throws an error', (assert) => {
+      assert.throws(F.createClassWithCachedSetter);
+    });
+
+    test('Cached decorator with arguments throws an error', function (assert) {
+      assert.throws(
+        F.createClassWithCachedDependentKeys,
+        /@cached\('firstName', 'lastName'\)/,
+        'the correct error is thrown'
+      );
+    });
+
+    test('Using @cached as a decorator factory throws an error', function (assert) {
+      assert.throws(
+        F.createClassWithCachedAsDecoratorFactory,
+        /@cached\(\)/,
+        'The correct error is thrown'
+      );
+    });
+  }
+});
diff --git a/packages/@glimmer/tracking/test/fixtures/babel.js b/packages/@glimmer/tracking/test/fixtures/babel.js
@@ -1,4 +1,4 @@
-import { tracked } from '@glimmer/tracking';
+import {cached, tracked} from '@glimmer/tracking';
 
 export class Tom {
   @tracked firstName = 'Tom';
@@ -16,7 +16,7 @@ class FrozenToran {
 
 Object.freeze(FrozenToran);
 
-export { FrozenToran };
+export {FrozenToran};
 
 export class PersonWithCount {
   @tracked _firstName = 'Tom';
@@ -100,6 +100,14 @@ export function createClassWithTrackedGetter() {
   return new PersonWithTrackedGetter();
 }
 
+export function createClassWithCachedProperty() {
+  class PersonWithCachedProperty {
+    @cached firstName = 'Tom';
+  }
+
+  return new PersonWithCachedProperty();
+}
+
 export function createClassWithTrackedSetter() {
   class PersonWithTrackedSetter {
     @tracked firstName = 'Tom';
@@ -115,6 +123,22 @@ export function createClassWithTrackedSetter() {
   return new PersonWithTrackedSetter();
 }
 
+export function createClassWithCachedSetter() {
+  class PersonWithCachedSetter {
+    @tracked firstName = 'Tom';
+    @tracked lastName;
+
+    @cached set fullName(fullName) {
+      const [firstName, lastName] = fullName.split(' ');
+      this.firstName = firstName;
+      this.lastName = lastName;
+    }
+  }
+
+  return new PersonWithCachedSetter();
+}
+
+
 export function createClassWithTrackedDependentKeys() {
   class DependentKeysAreCool {
     @tracked('firstName', 'lastName') fullName() {
@@ -127,6 +151,18 @@ export function createClassWithTrackedDependentKeys() {
   return new DependentKeysAreCool();
 }
 
+export function createClassWithCachedDependentKeys() {
+  class DependentKeysAreCool {
+    @cached('firstName', 'lastName') fullName() {
+      return `${this.firstName} ${this.lastName}`;
+    }
+
+    @tracked firstName = 'Tom';
+    @tracked lastName = 'Dale';
+  }
+  return new DependentKeysAreCool();
+}
+
 export function createClassWithTrackedAsDecoratorFactory() {
   class DependentKeysAreCool {
     @tracked() fullName() {
@@ -138,3 +174,15 @@ export function createClassWithTrackedAsDecoratorFactory() {
   }
   return new DependentKeysAreCool();
 }
+
+export function createClassWithCachedAsDecoratorFactory() {
+  class DependentKeysAreCool {
+    @cached() fullName() {
+      return `${this.firstName} ${this.lastName}`;
+    }
+
+    @tracked firstName = 'Tom';
+    @tracked lastName = 'Dale';
+  }
+  return new DependentKeysAreCool();
+}
diff --git a/packages/@glimmer/tracking/test/fixtures/typescript.ts b/packages/@glimmer/tracking/test/fixtures/typescript.ts
@@ -1,4 +1,4 @@
-import { tracked } from '@glimmer/tracking';
+import { cached, tracked } from '@glimmer/tracking';
 
 export class Tom {
   @tracked firstName = 'Tom';
@@ -100,6 +100,14 @@ export function createClassWithTrackedGetter(): any {
   return new PersonWithTrackedGetter();
 }
 
+export function createClassWithCachedProperty(): any {
+  class PersonWithCachedProperty {
+    @cached firstName = 'Tom';
+  }
+
+  return new PersonWithCachedProperty();
+}
+
 export function createClassWithTrackedSetter(): any {
   class PersonWithTrackedSetter {
     @tracked firstName = 'Tom';
@@ -116,6 +124,22 @@ export function createClassWithTrackedSetter(): any {
   return new PersonWithTrackedSetter();
 }
 
+export function createClassWithCachedSetter(): any {
+  class PersonWithCachedSetter {
+    @tracked firstName = 'Tom';
+    @tracked lastName: any;
+
+    // @ts-ignore
+    @cached set fullName(fullName) {
+      const [firstName, lastName] = fullName.split(' ');
+      this.firstName = firstName;
+      this.lastName = lastName;
+    }
+  }
+
+  return new PersonWithCachedSetter();
+}
+
 export function createClassWithTrackedDependentKeys(): any {
   class DependentKeysAreCool {
     // @ts-ignore
@@ -129,6 +153,19 @@ export function createClassWithTrackedDependentKeys(): any {
   return new DependentKeysAreCool();
 }
 
+export function createClassWithCachedDependentKeys(): any {
+  class DependentKeysAreCool {
+    // @ts-ignore
+    @cached('firstName', 'lastName') fullName() {
+      return `${this.firstName} ${this.lastName}`;
+    }
+
+    @tracked firstName = 'Tom';
+    @tracked lastName = 'Dale';
+  }
+  return new DependentKeysAreCool();
+}
+
 export function createClassWithTrackedAsDecoratorFactory(): any {
   class DependentKeysAreCool {
     // @ts-ignore
@@ -141,3 +178,16 @@ export function createClassWithTrackedAsDecoratorFactory(): any {
   }
   return new DependentKeysAreCool();
 }
+
+export function createClassWithCachedAsDecoratorFactory(): any {
+  class DependentKeysAreCool {
+    // @ts-ignore
+    @cached() fullName() {
+      return `${this.firstName} ${this.lastName}`;
+    }
+
+    @tracked firstName = 'Tom';
+    @tracked lastName = 'Dale';
+  }
+  return new DependentKeysAreCool();
+}
diff --git a/packages/@glimmer/tracking/test/index.ts b/packages/@glimmer/tracking/test/index.ts
@@ -1 +1,2 @@
 import './tracked-decorator-test';
+import './cached-decorator-test';