From 201085a6213d6297bf84b2da3d73e0ebdcce2e82 Mon Sep 17 00:00:00 2001
From: Charles Lowell <cowboyd@frontside.com>
Date: Fri, 13 Dec 2024 21:49:34 -0600
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8introduce=20v4=20action=20API?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The new action API is changing in order to be more in line with the
new Promise() constructor, so we want to communicate that change.

This adds the new action API to the old action API in a backwards
compatible manner via an overload, and then deprecates the old API.
---
 lib/instructions.ts | 59 ++++++++++++++++-----------------
 test/action.test.ts | 80 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 30 deletions(-)

diff --git a/lib/instructions.ts b/lib/instructions.ts
index 043d34f5e..ba49e26a8 100644
--- a/lib/instructions.ts
+++ b/lib/instructions.ts
@@ -54,52 +54,42 @@ function Suspend(frame: Frame) {
  * Create an {@link Operation} that can be either resolved (or rejected) with
  * a synchronous callback. This is the Effection equivalent of `new Promise()`.
  *
- * The action body is itself an operation that runs in a new scope that is
- * destroyed completely before program execution returns to the point where the
- * action was yielded to.
+ * The action body is a function that enters the effect, and returns a function that
+ * will be called to exit the action..
  *
  * For example:
  *
  * ```js
- * let five = yield* action(function*(resolve, reject) {
- *   setTimeout(() => {
+ * let five = yield* action((resolve, reject) => {
+ *   let timeout = setTimeout(() => {
  *     if (Math.random() > 5) {
  *       resolve(5)
  *     } else {
  *       reject(new Error("bad luck!"));
  *     }
  *   }, 1000);
- * });
- *
- * ```
- *
- * However, it is customary to explicitly {@link suspend} inside the body of the
- * action so that whenever the action resolves, appropriate cleanup code can
- * run. The preceeding example would be more correctly written as:
- *
- * ```js
- * let five = yield* action(function*(resolve) {
- *   let timeoutId = setTimeout(() => {
- *     if (Math.random() > 5) {
- *       resolve(5)
- *     } else {
- *       reject(new Error("bad luck!"));
- *     }
- *   }, 1000);
- *   try {
- *     yield* suspend();
- *   } finally {
- *     clearTimout(timeoutId);
- *   }
+ *   return () => clearTimeout(timeout);
  * });
  * ```
  *
  * @typeParam T - type of the action's result.
- * @param operation - body of the action
+ * @param body - enter and exit the action
  * @returns an operation producing the resolved value, or throwing the rejected error
  */
+export function action<T>(
+  enter: (resolve: Resolve<T>, reject: Reject) => () => void,
+): Operation<T>;
+
+/**
+ * @deprecated `action()` used with an operation will be removed in v4.
+ */
 export function action<T>(
   operation: (resolve: Resolve<T>, reject: Reject) => Operation<void>,
+): Operation<T>;
+export function action<T>(
+  operation:
+    | ((resolve: Resolve<T>, reject: Reject) => Operation<void>)
+    | ((resolve: Resolve<T>, reject: Reject) => () => void),
 ): Operation<T> {
   return instruction(function Action(frame) {
     return shift<Result<T>>(function* (k) {
@@ -119,8 +109,17 @@ export function action<T>(
       let reject: Reject = (error) => settle(Err(error));
 
       let child = frame.createChild(function* () {
-        yield* operation(resolve, reject);
-        yield* suspend();
+        let iterable = operation(resolve, reject);
+        if (typeof iterable === "function") {
+          try {
+            yield* suspend();
+          } finally {
+            iterable();
+          }
+        } else {
+          yield* iterable;
+          yield* suspend();
+        }
       });
 
       yield* reset(function* () {
diff --git a/test/action.test.ts b/test/action.test.ts
index f2287e37d..657a3cac7 100644
--- a/test/action.test.ts
+++ b/test/action.test.ts
@@ -119,4 +119,84 @@ describe("action", () => {
     });
     expect(didReach).toEqual(false);
   });
+
+  describe("v4 api", () => {
+    it("can resolve", async () => {
+      let didClear = false;
+      let task = run(() =>
+        action<number>((resolve) => {
+          let timeout = setTimeout(() => resolve(42), 5);
+          return () => {
+            didClear = true;
+            clearTimeout(timeout);
+          };
+        })
+      );
+
+      await expect(task).resolves.toEqual(42);
+      expect(didClear).toEqual(true);
+    });
+
+    it("can reject", async () => {
+      let didClear = false;
+      let error = new Error("boom");
+      let task = run(() =>
+        action<number>((_, reject) => {
+          let timeout = setTimeout(() => reject(error), 5);
+          return () => {
+            didClear = true;
+            clearTimeout(timeout);
+          };
+        })
+      );
+
+      await expect(task).rejects.toEqual(error);
+      expect(didClear).toEqual(true);
+    });
+
+    it("can resolve without ever suspending", async () => {
+      let result = await run(() =>
+        action<string>((resolve) => {
+          resolve("hello");
+          return () => {};
+        })
+      );
+
+      expect(result).toEqual("hello");
+    });
+
+    it("can reject without ever suspending", async () => {
+      let error = new Error("boom");
+      let task = run(() =>
+        action((_, reject) => {
+          reject(error);
+          return () => {};
+        })
+      );
+      await expect(task).rejects.toEqual(error);
+    });
+
+    it("fails if the operation fails", async () => {
+      let task = run(() =>
+        action(() => {
+          throw new Error("boom");
+        })
+      );
+      await expect(task).rejects.toHaveProperty("message", "boom");
+    });
+
+    it("fails if the shutdown fails", async () => {
+      let error = new Error("boom");
+      let task = run(() =>
+        action((resolve) => {
+          let timeout = setTimeout(resolve, 5);
+          return () => {
+            clearTimeout(timeout);
+            throw error;
+          };
+        })
+      );
+      await expect(task).rejects.toEqual(error);
+    });
+  });
 });