From 17fac4d00822a0f17133c8f410cbe1f506f8049a Mon Sep 17 00:00:00 2001 From: Eryn Lynn Date: Sat, 28 Sep 2019 00:13:30 -0400 Subject: [PATCH] Make all/race cancel correctly --- CHANGELOG.md | 4 ++++ lib/README.md | 4 +++- lib/init.lua | 41 +++++++++++++++++++++++++++++++---------- lib/init.spec.lua | 15 +++++++++------ rotriever.toml | 2 +- 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e6bdd..ec6d5a6 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/lib/README.md b/lib/README.md index 3a89d8c..a86da21 100644 --- a/lib/README.md +++ b/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>" returns: Promise> @@ -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>" returns: Promise diff --git a/lib/init.lua b/lib/init.lua index b73c02a..779bfee 100644 --- a/lib/init.lua +++ b/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 diff --git a/lib/init.spec.lua b/lib/init.spec.lua index 24a0108..c9accd9 100644 --- a/lib/init.spec.lua +++ b/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() diff --git a/rotriever.toml b/rotriever.toml index d29a50b..a62514c 100644 --- a/rotriever.toml +++ b/rotriever.toml @@ -1,6 +1,6 @@ [package] name = "roblox-lua-promise" -version = "2.4.0" +version = "2.4.1" author = "evaera" content_root = "lib"