Merge pull request #81 from NullVoxPopuli/fix-async-functions-in-production

feat(functions): BREAKING CHANGE: all functions async

Author: NullVoxPopuli

.github/workflows/ci.yml

@@ -71,7 +71,7 @@ jobs:
           - ember-3.26
           - ember-release
           - ember-beta
-          - ember-canary
+          # - ember-canary
           - embroider-safe
           - embroider-optimized
     steps:

README.md

@@ -180,6 +180,7 @@ class StarWarsInfo {
   }, () => [this.ids /* defined somewhere */])
 }
 ```
+> `characters` would be accessed via `this.info.value.characters` in the `StarWarsInfo` class
 
 While this example is a bit contrived, hopefully it demonstrates how the `state` arg
 works. During the first invocation, `state` is falsey, allowing the rest of the
@@ -193,7 +194,8 @@ as the function doesn't interact with `this`).
 In this example, where the function is `async`, the "value" of `info.value` is `undefined` until the
 function completes.
 
-If a function is synchronous, you can avoid the thunk altogether,
+To help prevent accidental async footguns, even if a function is synchronous, it is still ran
+asynchronously, therefor, the thunk cannot be avoided.
 
 ```ts
 class MyClass {
@@ -205,14 +207,14 @@ class MyClass {
 }
 ```
 
-`this.info.value` will be `6`
+`this.info.value` will be  `undefined`, then `6` and will not change when `num` changes.
 
 
 ### Thunks
 
 With the exception of the `useResource` + `class` combination, all Thunks are optional.
-The main caveat is that if you want your resource to update, you _must_ consume the tracked
-properties during setup / initial execution.
+The main caveat is that if your resources will not update without a thunk -- or consuming
+tracked data within setup / initialization (which is done for you with `useFunction`).
 
 
  - The thunk is "just a function" that allows tracked data to be lazily consumed by the resource.

addon/-private/resources/function-runner.ts

@@ -54,6 +54,11 @@ export class FunctionRunner<
     this.update();
   }
 
+  /**
+   * NOTE: there is no reliable way to determine if a function is async before the function is ran.
+   *   - we can't use fun[Symbol.toStringtag] === 'AsyncFunction' because minifiers may remove the
+   *     async keyword
+   */
   update() {
     /**
      * NOTE: All positional args are consumed
@@ -63,40 +68,35 @@ export class FunctionRunner<
     }
 
     const fun = this[FUNCTION_TO_RUN];
-    // TS doesn't add the default function symbols to function types...
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    const isAsync = (fun as any)[Symbol.toStringTag] === 'AsyncFunction';
+
     /**
      * Do not access "value" directly in this function. You'll have infinite re-rendering errors
      */
     const previous = this[SECRET_VALUE];
 
-    if (isAsync) {
-      const asyncWaiter = async () => {
-        // in case the async function tries to consume things on the parent `this`,
-        // be sure we start with a fresh frame
-        await new Promise((resolve) => schedule('afterRender', resolve));
+    const asyncWaiter = async () => {
+      // in case the async function tries to consume things on the parent `this`,
+      // be sure we start with a fresh frame
+      await new Promise((resolve) => schedule('afterRender', resolve));
 
-        if (isDestroying(this) || isDestroyed(this)) {
-          return;
-        }
+      if (isDestroying(this) || isDestroyed(this)) {
+        return;
+      }
 
-        const value = await fun(previous, ...this.funArgs);
+      const value = await fun(previous, ...this.funArgs);
 
-        if (isDestroying(this) || isDestroyed(this)) {
-          return;
-        }
+      if (isDestroying(this) || isDestroyed(this)) {
+        return;
+      }
 
-        this[SECRET_VALUE] = value;
-        this[HAS_RUN] = true;
-        dirty(this, SECRET_VALUE);
-      };
+      this[SECRET_VALUE] = value;
+      this[HAS_RUN] = true;
+      dirty(this, SECRET_VALUE);
+    };
 
-      waitForPromise(asyncWaiter());
-
-      return;
-    }
+    waitForPromise(asyncWaiter());
 
-    this[SECRET_VALUE] = fun(previous, ...this.funArgs) as Return;
+    // If we ever want to bring sync-support back:
+    // this[SECRET_VALUE] = fun(previous, ...this.funArgs) as Return;
   }
 }

tests/unit/use-function-test.ts

@@ -56,24 +56,37 @@ module('useFunction', function () {
 
         data = useFunction(
           this,
-          (previous: number, count: number) => count * (previous || 1),
+          (previous: number, count: number) => {
+            return count * (previous || 1);
+          },
           () => [this.count]
         );
       }
 
       let foo = new Test();
 
+      assert.equal(foo.data.value, undefined);
+      await settled();
+
       assert.equal(foo.data.value, 1);
 
       foo.count = 2;
+      foo.data.value;
       await settled();
 
       assert.equal(foo.data.value, 2);
 
       foo.count = 6;
+      foo.data.value;
       await settled();
 
       assert.equal(foo.data.value, 12);
+
+      foo.count = 7;
+      foo.data.value;
+      await settled();
+
+      assert.equal(foo.data.value, 84);
     });
 
     test('it works with async functions', async function (assert) {
@@ -252,7 +265,10 @@ module('useFunction', function () {
 
       await click('button');
 
-      assert.dom('out').hasText('4');
+      // NOTE: this may be unintuitive because of the direct access to this.count.
+      //       It's important to know that the function is invoked async, so there is no way to
+      //       know that this.count should cause re-invocations without the thunk
+      assert.dom('out').hasText('2');
     });
   });
 });