fix Promise.finally() to return new promise via .then() (#1696)

RealColdFry · web-flow · commit 8c856f1d4f0c · 2026-03-28T13:58:01.000+01:00
* fix Promise.finally() to return new promise via .then() * simplify Promise.finally test to use expectToMatchJsResult Remove redundant tests that relied on synchronous polyfill behavior differing from JS. Keep only the identity check which matches JS. * add tests for Promise.finally value preservation and rejection passthrough Fix issue reference (#1660, not #1659).
diff --git a/src/lualib/Promise.ts b/src/lualib/Promise.ts
@@ -45,7 +45,6 @@ export class __TS__Promise<T> implements Promise<T> {
 
     private fulfilledCallbacks: Array<PromiseResolve<T>> = [];
     private rejectedCallbacks: PromiseReject[] = [];
-    private finallyCallbacks: Array<() => void> = [];
 
     // @ts-ignore
     public [Symbol.toStringTag]: string; // Required to implement interface, no output Lua
@@ -124,16 +123,23 @@ export class __TS__Promise<T> implements Promise<T> {
     }
 
     // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/finally
+    // Delegates to .then() so that a new Promise is returned (per ES spec §27.2.5.3)
+    // and the original fulfillment value / rejection reason is preserved.
     public finally(onFinally?: () => void): Promise<T> {
-        if (onFinally) {
-            this.finallyCallbacks.push(onFinally);
-
-            if (this.state !== PromiseState.Pending) {
-                // If promise already resolved or rejected, immediately fire finally callback
-                onFinally();
-            }
-        }
-        return this;
+        return this.then(
+            onFinally
+                ? (value: T): T => {
+                      onFinally();
+                      return value;
+                  }
+                : undefined,
+            onFinally
+                ? (reason: any): never => {
+                      onFinally();
+                      throw reason;
+                  }
+                : undefined
+        );
     }
 
     private resolve(value: T | PromiseLike<T>): void {
@@ -168,25 +174,13 @@ export class __TS__Promise<T> implements Promise<T> {
 
     private invokeCallbacks<T>(callbacks: ReadonlyArray<(value: T) => void>, value: T): void {
         const callbacksLength = callbacks.length;
-        const finallyCallbacks = this.finallyCallbacks;
-        const finallyCallbacksLength = finallyCallbacks.length;
 
         if (callbacksLength !== 0) {
             for (const i of $range(1, callbacksLength - 1)) {
                 callbacks[i - 1](value);
             }
             // Tail call optimization for a common case.
-            if (finallyCallbacksLength === 0) {
-                return callbacks[callbacksLength - 1](value);
-            }
-            callbacks[callbacksLength - 1](value);
-        }
-
-        if (finallyCallbacksLength !== 0) {
-            for (const i of $range(1, finallyCallbacksLength - 1)) {
-                finallyCallbacks[i - 1]();
-            }
-            return finallyCallbacks[finallyCallbacksLength - 1]();
+            return callbacks[callbacksLength - 1](value);
         }
     }
 
diff --git a/test/unit/builtins/promise.spec.ts b/test/unit/builtins/promise.spec.ts
@@ -1323,3 +1323,51 @@ describe("Promise.race", () => {
             });
     });
 });
+
+// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1660
+describe("Promise.finally", () => {
+    test("returns a different promise instance", () => {
+        util.testFunction`
+            const p1 = new Promise(() => {});
+            const p2 = p1.finally();
+            return p1 === p2;
+        `.expectToMatchJsResult();
+    });
+
+    test("preserves fulfillment value", () => {
+        util.testFunction`
+            const result = Promise.resolve(42).finally(() => {}) as any;
+            return result.value;
+        `.expectToEqual(42);
+    });
+
+    test("preserves rejection reason", () => {
+        util.testFunction`
+            const result = Promise.reject("err").finally(() => {}) as any;
+            return result.rejectionReason;
+        `.expectToEqual("err");
+    });
+
+    test("callback executes on fulfillment", () => {
+        util.testFunction`
+            let called = false;
+            Promise.resolve(1).finally(() => { called = true; });
+            return called;
+        `.expectToEqual(true);
+    });
+
+    test("callback executes on rejection", () => {
+        util.testFunction`
+            let called = false;
+            Promise.reject("err").finally(() => { called = true; });
+            return called;
+        `.expectToEqual(true);
+    });
+
+    test("finally with undefined callback", () => {
+        util.testFunction`
+            const result = Promise.resolve(99).finally(undefined) as any;
+            return result.value;
+        `.expectToEqual(99);
+    });
+});
