Make all/race cancel correctly

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 2.4.1
+
+- Fix issue with Promise.race/all always cancelling instead of only cancelling if the Promise has no other consumers
+
 # 2.4.0
 
 - `Promise.is` now only checks if the object is "andThennable" (has an `andThen` method).

lib/README.md

@@ -174,6 +174,8 @@ docs:
           * is resolved after all input promises resolve.
           * is rejected if ANY input promises reject.
         Note: Only the first return value from each promise will be present in the resulting array.
+
+        After any input Promise rejects, all other input Promises that are still pending will be cancelled if they have no other consumers.
       static: true
       params: "promises: array<Promise<T>>"
       returns: Promise<array<T>>
@@ -181,7 +183,7 @@ docs:
       desc: |
         Accepts an array of Promises and returns a new promise that is resolved or rejected as soon as any Promise in the array resolves or rejects.
 
-        All other Promises that don't win the race will be cancelled.
+        All other Promises that don't win the race will be cancelled if they have no other consumers.
       static: true
       params: "promises: array<Promise<T>>"
       returns: Promise<T>

lib/init.lua

@@ -227,10 +227,12 @@ function Promise.all(promises)
 	return Promise.new(function(resolve, reject)
 		-- An array to contain our resolved values from the given promises.
 		local resolvedValues = {}
+		local newPromises = {}
 
 		-- Keep a count of resolved promises because just checking the resolved
 		-- values length wouldn't account for promises that resolve with nil.
 		local resolvedCount = 0
+		local finalized = false
 
 		-- Called when a single value is resolved and resolves if all are done.
 		local function resolveOne(i, ...)
@@ -245,13 +247,25 @@ function Promise.all(promises)
 		-- We can assume the values inside `promises` are all promises since we
 		-- checked above.
 		for i = 1, #promises do
-			promises[i]:andThen(
-				function(...)
-					resolveOne(i, ...)
-				end,
-				function(...)
-					reject(...)
-				end
+			if finalized then
+				break
+			end
+
+			table.insert(
+				newPromises,
+				promises[i]:andThen(
+					function(...)
+						resolveOne(i, ...)
+					end,
+					function(...)
+						for _, promise in ipairs(newPromises) do
+							promise:cancel()
+						end
+						finalized = true
+
+						reject(...)
+					end
+				)
 			)
 		end
 	end)
@@ -269,9 +283,11 @@ function Promise.race(promises)
 	end
 
 	return Promise.new(function(resolve, reject, onCancel)
+		local newPromises = {}
+
 		local function finalize(callback)
 			return function (...)
-				for _, promise in ipairs(promises) do
+				for _, promise in ipairs(newPromises) do
 					promise:cancel()
 				end
 
@@ -279,10 +295,15 @@ function Promise.race(promises)
 			end
 		end
 
-		onCancel(finalize(reject))
+		if onCancel(finalize(reject)) then
+			return
+		end
 
 		for _, promise in ipairs(promises) do
-			promise:andThen(finalize(resolve), finalize(reject))
+			table.insert(
+				newPromises,
+				promise:andThen(finalize(resolve), finalize(reject))
+			)
 		end
 	end)
 end

lib/init.spec.lua

@@ -480,8 +480,8 @@ return function()
 
 			expect(combinedPromise:getStatus()).to.equal(Promise.Status.Started)
 
-			resolveB("foo", "bar")
 			rejectA("baz", "qux")
+			resolveB("foo", "bar")
 
 			local resultLength, result = pack(combinedPromise:_unwrap())
 			local success, first, second = unpack(result, 1, resultLength)
@@ -490,6 +490,7 @@ return function()
 			expect(success).to.equal(false)
 			expect(first).to.equal("baz")
 			expect(second).to.equal("qux")
+			expect(b:getStatus()).to.equal(Promise.Status.Cancelled)
 		end)
 
 		it("should not resolve if resolved after rejecting", function()
@@ -574,10 +575,11 @@ return function()
 		end)
 
 		it("should cancel other promises", function()
+			local promise = Promise.new(function() end)
+			promise:andThen(function() end)
 			local promises = {
-				Promise.new(function(resolve)
-					-- resolve(1)
-				end),
+				promise,
+				Promise.new(function() end),
 				Promise.new(function(resolve)
 					resolve(2)
 				end)
@@ -587,8 +589,9 @@ return function()
 
 			expect(promise:getStatus()).to.equal(Promise.Status.Resolved)
 			expect(promise._values[1]).to.equal(2)
-			expect(promises[1]:getStatus()).to.equal(Promise.Status.Cancelled)
-			expect(promises[2]:getStatus()).to.equal(Promise.Status.Resolved)
+			expect(promises[1]:getStatus()).to.equal(Promise.Status.Started)
+			expect(promises[2]:getStatus()).to.equal(Promise.Status.Cancelled)
+			expect(promises[3]:getStatus()).to.equal(Promise.Status.Resolved)
 		end)
 
 		it("should error if a non-array table is passed in", function()

rotriever.toml

@@ -1,6 +1,6 @@
 [package]
 name = "roblox-lua-promise"
-version = "2.4.0"
+version = "2.4.1"
 author = "evaera"
 content_root = "lib"