From 82576fb5b7b8077b55980100011b7f97ac84b945 Mon Sep 17 00:00:00 2001 From: Aldwin Vlasblom Date: Thu, 30 Aug 2018 13:33:47 +0200 Subject: [PATCH 1/2] Remove Future.finally and its aliases The finally function has long been surpassed by Future.hook, and has several problems: 1. An undocumented behaviour where it will run *and cancel* your cleanup Future upon cancellation. This once was intended, and copied to Future.hook, but then it was decided to be bad behaviour and removed from Future.hook. It stuck around in Future.finally. 2. It doesn't clean up when the Future crashes. 3. It's not very useful for any other kind of control flow. --- README.md | 54 +--------------- index.d.ts | 10 --- src/dispatchers/index.mjs | 1 - src/dispatchers/lastly.mjs | 14 ---- src/future.mjs | 11 ---- test.js | 1 - test/2.transformation.test.mjs | 28 -------- test/5.finally.test.mjs | 115 --------------------------------- 8 files changed, 2 insertions(+), 232 deletions(-) delete mode 100644 src/dispatchers/lastly.mjs delete mode 100644 test/5.finally.test.mjs diff --git a/README.md b/README.md index 06066b87..7de43836 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,6 @@ for sponsoring the project. - [`ap`: Combine the success values of multiple Futures using a function](#ap) - [`and`: Logical *and* for Futures](#and) - [`or`: Logical *or* for Futures](#or) -- [`finally`: Run a Future after the previous settles](#finally) - [`race`: Race two Futures against each other](#race) - [`both`: Await both success values from two Futures](#both) - [`parallel`: Await all success values from many Futures](#parallel) @@ -1036,7 +1035,7 @@ Returns a new Future which either rejects with the first rejection reason, or resolves with the last resolution value once and if both Futures resolve. We can use it if we want a computation to run only after another has succeeded. -See also [`or`](#or) and [`finally`](#finally). +See also [`or`](#or). ```js Future.after(300, null) @@ -1072,7 +1071,7 @@ Returns a new Future which either resolves with the first resolution value, or rejects with the last rejection value once and if both Futures reject. We can use it if we want a computation to run only if another has failed. -See also [`and`](#and) and [`finally`](#finally). +See also [`and`](#and). ```js Future.rejectAfter(300, new Error('Failed')) @@ -1091,55 +1090,6 @@ any([Future.reject(1), Future.after(20, 2), Future.of(3)]).value(console.log); //> 2 ``` -#### finally - -
finally :: Future a c -> Future a b -> Future a b - -```hs -finally :: Future a c -> Future a b -> Future a b -lastly :: Future a c -> Future a b -> Future a b -Future.prototype.finally :: Future a b ~> Future a c -> Future a b -Future.prototype.lastly :: Future a b ~> Future a c -> Future a b -``` - -
- -Run a second Future after the first settles (successfully or unsuccessfully). -Rejects with the rejection reason from the first or second Future, or resolves -with the resolution value from the first Future. We can use this when we want -a computation to run after another settles, successfully or unsuccessfully. - -If you're looking to clean up resources after running a computation which -acquires them, you should use [`hook`](#hook), which has many more fail-safes -in place. - -This function has an alias `lastly`, for environments where `finally` is -reserved. - -See also [`and`](#and) and [`or`](#or). - -```js -Future.of('Hello') -.finally(Future.of('All done!').map(console.log)) -.fork(console.error, console.log); -//> "All done!" -//> "Hello" -``` - -Note that the *first* Future is given as the *last* argument to `Future.finally()`: - -```js -var program = S.pipe([ - Future.of, - Future.finally(Future.of('All done!').map(console.log)), - Future.fork(console.error, console.log) -]); - -program('Hello'); -//> "All done!" -//> "Hello" -``` - ### Consuming Futures #### fork diff --git a/index.d.ts b/index.d.ts index ec050573..4b7f4244 100644 --- a/index.d.ts +++ b/index.d.ts @@ -87,9 +87,6 @@ declare module 'fluture' { /** Attempt to extract the resolution value. See https://github.com/fluture-js/Fluture#extractright */ extractRight(): Array - /** Set up a cleanup Future to run after this one is done. See https://github.com/fluture-js/Fluture#finally */ - finally(cleanup: FutureInstance): FutureInstance - /** Fold both branches into the resolution branch. See https://github.com/fluture-js/Fluture#fold */ fold(lmapper: (reason: L) => RB, rmapper: (value: R) => RB): FutureInstance @@ -99,9 +96,6 @@ declare module 'fluture' { /** Fork with exception recovery. See https://github.com/fluture-js/Fluture#forkCatch */ forkCatch(recover: RecoverFunction, reject: RejectFunction, resolve: ResolveFunction): Cancel - /** Set up a cleanup Future to run after this one is done. See https://github.com/fluture-js/Fluture#finally */ - lastly(cleanup: FutureInstance): FutureInstance - /** Map over the resolution value in this Future. See https://github.com/fluture-js/Fluture#map */ map(mapper: (value: R) => RB): FutureInstance @@ -254,10 +248,6 @@ declare module 'fluture' { /** Returns true for Futures that will certainly never settle. See https://github.com/fluture-js/Fluture#isnever */ export function isNever(value: any): boolean - /** Set up a cleanup Future to run after the given action has settled. See https://github.com/fluture-js/Fluture#lastly */ - export function lastly(cleanup: FutureInstance, action: FutureInstance): FutureInstance - export function lastly(cleanup: FutureInstance): (action: FutureInstance) => FutureInstance - /** Map over the resolution value of the given Future. See https://github.com/fluture-js/Fluture#map */ export function map(mapper: (value: RA) => RB, source: FutureInstance): FutureInstance export function map(mapper: (value: RA) => RB): (source: FutureInstance) => FutureInstance diff --git a/src/dispatchers/index.mjs b/src/dispatchers/index.mjs index fdee88e1..06e2b7b0 100644 --- a/src/dispatchers/index.mjs +++ b/src/dispatchers/index.mjs @@ -6,7 +6,6 @@ export {chain} from './chain'; export {mapRej} from './map-rej'; export {chainRej} from './chain-rej'; -export {lastly, lastly as finally} from './lastly'; export {and} from './and'; export {both} from './both'; diff --git a/src/dispatchers/lastly.mjs b/src/dispatchers/lastly.mjs deleted file mode 100644 index 1dfe71fa..00000000 --- a/src/dispatchers/lastly.mjs +++ /dev/null @@ -1,14 +0,0 @@ -import {isFuture} from '../future'; -import {partial1} from '../internal/utils'; -import {throwInvalidFuture} from '../internal/throw'; - -function lastly$right(right, left){ - if(!isFuture(left)) throwInvalidFuture('Future.finally', 1, left); - return left.finally(right); -} - -export function lastly(right, left){ - if(!isFuture(right)) throwInvalidFuture('Future.finally', 0, right); - if(arguments.length === 1) return partial1(lastly$right, right); - return lastly$right(right, left); -} diff --git a/src/future.mjs b/src/future.mjs index 77377ac1..eb18cca3 100644 --- a/src/future.mjs +++ b/src/future.mjs @@ -616,17 +616,6 @@ defineBimapperAction('fold', { rejected: function FoldAction$rejected(x){ return mapWith(this.lmapper, resolve, x) } }); -var finallyAction = { - cancel: function FinallyAction$cancel(){ this.other._interpret(noop, noop, noop)() }, - rejected: function FinallyAction$rejected(x){ return this.other._and(new Rejected(x)) }, - resolved: function FinallyAction$resolved(x){ - return this.other._map(function FoldAction$resolved$mapper(){ return x }); - } -}; - -defineOtherAction('finally', finallyAction); -defineOtherAction('lastly', finallyAction); - defineOtherAction('and', { resolved: returnOther }); diff --git a/test.js b/test.js index f2e408a7..4e7febad 100644 --- a/test.js +++ b/test.js @@ -21,7 +21,6 @@ import './test/5.chain.test.mjs'; import './test/5.encase-n.test.mjs'; import './test/5.encase-p.test.mjs'; import './test/5.encase.test.mjs'; -import './test/5.finally.test.mjs'; import './test/5.fold.test.mjs'; import './test/5.hook.test.mjs'; import './test/5.map-rej.test.mjs'; diff --git a/test/2.transformation.test.mjs b/test/2.transformation.test.mjs index 8d94525f..df0bb8e1 100644 --- a/test/2.transformation.test.mjs +++ b/test/2.transformation.test.mjs @@ -340,34 +340,6 @@ describe('Transformation', function (){ }); - describe('finally', function (){ - - var seq = dummy.finally(dummy); - - describe('#_interpret()', function (){ - - it('runs the action', function (){ - return assertResolved(seq, 'resolved'); - }); - - it('runs the other if the left rejects', function (done){ - var other = Future(function (){done()}); - var m = new Transformation(rejected, nil).finally(other); - m._interpret(done, noop, noop); - }); - - }); - - describe('#toString()', function (){ - - it('returns code to create the same data-structure', function (){ - expect(seq.toString()).to.equal('Future.of("resolved").finally(Future.of("resolved"))'); - }); - - }); - - }); - describe('in general', function (){ describe('#_interpret()', function (){ diff --git a/test/5.finally.test.mjs b/test/5.finally.test.mjs deleted file mode 100644 index c4167d78..00000000 --- a/test/5.finally.test.mjs +++ /dev/null @@ -1,115 +0,0 @@ -import chai from 'chai'; -import {Future, lastly, of, reject} from '../index.mjs'; -import * as U from './util'; -import * as F from './futures'; -import type from 'sanctuary-type-identifiers'; - -var expect = chai.expect; - -var testInstance = function (fin){ - - it('is considered a member of fluture/Fluture', function (){ - expect(type(fin(of(1), of(2)))).to.equal(Future['@@type']); - }); - - describe('#_interpret()', function (){ - - it('runs the second Future when the first resolves', function (done){ - fin(of(1), of(null).map(done))._interpret(done, U.noop, U.noop); - }); - - it('runs the second Future when the first rejects', function (done){ - fin(reject(1), of(null).map(done))._interpret(done, U.noop, U.noop); - }); - - it('resolves with the resolution value of the first', function (){ - var actual = fin(of(1), of(2)); - return U.assertResolved(actual, 1); - }); - - it('rejects with the rejection reason of the first if the second resolves', function (){ - var actual = fin(reject(1), of(2)); - return U.assertRejected(actual, 1); - }); - - it('always rejects with the rejection reason of the second', function (){ - var actualResolved = fin(of(1), reject(2)); - var actualRejected = fin(reject(1), reject(2)); - return Promise.all([ - U.assertRejected(actualResolved, 2), - U.assertRejected(actualRejected, 2) - ]); - }); - - it('does nothing after being cancelled', function (done){ - fin(F.resolvedSlow, F.resolved)._interpret(done, U.failRej, U.failRes)(); - fin(F.resolved, F.resolvedSlow)._interpret(done, U.failRej, U.failRes)(); - fin(F.rejectedSlow, F.rejected)._interpret(done, U.failRej, U.failRes)(); - fin(F.rejected, F.rejectedSlow)._interpret(done, U.failRej, U.failRes)(); - setTimeout(done, 25); - }); - - it('immediately runs and cancels the disposal Future when cancelled early', function (done){ - var cancel = fin(F.resolvedSlow, Future(function (){ return function (){ return done() } }))._interpret(done, U.failRej, U.failRes); - setTimeout(cancel, 10); - }); - - }); - -}; - -describe('finally()', function (){ - - it('is a curried binary function', function (){ - expect(lastly).to.be.a('function'); - expect(lastly.length).to.equal(2); - expect(lastly(of(1))).to.be.a('function'); - }); - - it('throws when not given a Future as first argument', function (){ - var f = function (){ return lastly(1) }; - expect(f).to.throw(TypeError, /Future.*first/); - }); - - it('throws when not given a Future as second argument', function (){ - var f = function (){ return lastly(of(1), 1) }; - expect(f).to.throw(TypeError, /Future.*second/); - }); - - testInstance(function (a, b){ return lastly(b, a) }); - -}); - -describe('Future#finally()', function (){ - - it('throws when invoked out of context', function (){ - var f = function (){ return of(1).finally.call(null, of(1)) }; - expect(f).to.throw(TypeError, /Future/); - }); - - it('throws TypeError when not given a Future', function (){ - var xs = [NaN, {}, [], 1, 'a', new Date, undefined, null, function (x){ return x }]; - var fs = xs.map(function (x){ return function (){ return of(1).finally(x) } }); - fs.forEach(function (f){ return expect(f).to.throw(TypeError, /Future/) }); - }); - - testInstance(function (a, b){ return a.finally(b) }); - -}); - -describe('Future#lastly()', function (){ - - it('throws when invoked out of context', function (){ - var f = function (){ return of(1).lastly.call(null, of(1)) }; - expect(f).to.throw(TypeError, /Future/); - }); - - it('throws TypeError when not given a Future', function (){ - var xs = [NaN, {}, [], 1, 'a', new Date, undefined, null, function (x){ return x }]; - var fs = xs.map(function (x){ return function (){ return of(1).lastly(x) } }); - fs.forEach(function (f){ return expect(f).to.throw(TypeError, /Future/) }); - }); - - testInstance(function (a, b){ return a.lastly(b) }); - -}); From 17449fad156bf2408b1fd17b6b9b08c10a7ceec0 Mon Sep 17 00:00:00 2001 From: Aldwin Vlasblom Date: Thu, 30 Aug 2018 13:57:14 +0200 Subject: [PATCH 2/2] Add Future.assume to fill the gap that Future.finally left I've personally been using Future.finally *only* to shorten this: someFuture .chain(_ => Future.reject('Kaputt')) .chainRej(_ => Future.reject('Kaputt')) To this: someFuture.finally(Future.reject('Kaputt')) Now that finally is gone, I'll replace it by a function which does this job, and does it better (it works for resolved Futures as well). --- README.md | 46 +++++++++++++- index.d.ts | 7 +++ src/dispatchers/assume.mjs | 14 +++++ src/dispatchers/index.mjs | 1 + src/future.mjs | 5 ++ test.js | 1 + test/2.transformation.test.mjs | 28 +++++++++ test/5.assume.test.mjs | 109 +++++++++++++++++++++++++++++++++ 8 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 src/dispatchers/assume.mjs create mode 100644 test/5.assume.test.mjs diff --git a/README.md b/README.md index 7de43836..68084f1e 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,7 @@ for sponsoring the project. - [`ap`: Combine the success values of multiple Futures using a function](#ap) - [`and`: Logical *and* for Futures](#and) - [`or`: Logical *or* for Futures](#or) +- [`assume`: Continue with another Future after the previous settles](#assume) - [`race`: Race two Futures against each other](#race) - [`both`: Await both success values from two Futures](#both) - [`parallel`: Await all success values from many Futures](#parallel) @@ -1035,7 +1036,7 @@ Returns a new Future which either rejects with the first rejection reason, or resolves with the last resolution value once and if both Futures resolve. We can use it if we want a computation to run only after another has succeeded. -See also [`or`](#or). +See also [`or`](#or) and [`assume`](#assume). ```js Future.after(300, null) @@ -1071,7 +1072,7 @@ Returns a new Future which either resolves with the first resolution value, or rejects with the last rejection value once and if both Futures reject. We can use it if we want a computation to run only if another has failed. -See also [`and`](#and). +See also [`and`](#and) and [`assume`](#assume). ```js Future.rejectAfter(300, new Error('Failed')) @@ -1090,6 +1091,47 @@ any([Future.reject(1), Future.after(20, 2), Future.of(3)]).value(console.log); //> 2 ``` +#### assume + +
assume :: Future c d -> Future a b -> Future c d + +```hs +assume :: Future c d -> Future a b -> Future c d +Future.prototype.assume :: Future a b ~> Future c d -> Future c d +``` + +
+ +Run a second Future after the first settles (successfully or unsuccessfully), +assuming its state. + +See also [`and`](#and) and [`or`](#or). + +```js +Future.of('Hello') +.assume(Future.of('World!')) +.fork(console.error, console.log); +//> "World!" + +Future.reject('Hello') +.assume(Future.of('World!')) +.fork(console.error, console.log); +//> "World!" +``` + +Note that the *first* Future is given as the *last* argument to `Future.assume()`: + +```js +var program = S.pipe([ + Future.of, + Future.assume(Future.of('World!')), + Future.fork(console.error, console.log) +]); + +program('Hello'); +//> "World!" +``` + ### Consuming Futures #### fork diff --git a/index.d.ts b/index.d.ts index 4b7f4244..3f5c909c 100644 --- a/index.d.ts +++ b/index.d.ts @@ -87,6 +87,9 @@ declare module 'fluture' { /** Attempt to extract the resolution value. See https://github.com/fluture-js/Fluture#extractright */ extractRight(): Array + /** Continue with another Future after this one settles. See https://github.com/fluture-js/Fluture#assume */ + assume(other: FutureInstance): FutureInstance + /** Fold both branches into the resolution branch. See https://github.com/fluture-js/Fluture#fold */ fold(lmapper: (reason: L) => RB, rmapper: (value: R) => RB): FutureInstance @@ -248,6 +251,10 @@ declare module 'fluture' { /** Returns true for Futures that will certainly never settle. See https://github.com/fluture-js/Fluture#isnever */ export function isNever(value: any): boolean + /** Continue with another Future after the previous settles. See https://github.com/fluture-js/Fluture#assume */ + export function assume(other: FutureInstance, source: FutureInstance): FutureInstance + export function assume(other: FutureInstance): (source: FutureInstance) => FutureInstance + /** Map over the resolution value of the given Future. See https://github.com/fluture-js/Fluture#map */ export function map(mapper: (value: RA) => RB, source: FutureInstance): FutureInstance export function map(mapper: (value: RA) => RB): (source: FutureInstance) => FutureInstance diff --git a/src/dispatchers/assume.mjs b/src/dispatchers/assume.mjs new file mode 100644 index 00000000..bc07d726 --- /dev/null +++ b/src/dispatchers/assume.mjs @@ -0,0 +1,14 @@ +import {isFuture} from '../future'; +import {partial1} from '../internal/utils'; +import {throwInvalidFuture} from '../internal/throw'; + +function assume$right(right, left){ + if(!isFuture(left)) throwInvalidFuture('Future.assume', 1, left); + return left.assume(right); +} + +export function assume(right, left){ + if(!isFuture(right)) throwInvalidFuture('Future.assume', 0, right); + if(arguments.length === 1) return partial1(assume$right, right); + return assume$right(right, left); +} diff --git a/src/dispatchers/index.mjs b/src/dispatchers/index.mjs index 06e2b7b0..49f5604c 100644 --- a/src/dispatchers/index.mjs +++ b/src/dispatchers/index.mjs @@ -6,6 +6,7 @@ export {chain} from './chain'; export {mapRej} from './map-rej'; export {chainRej} from './chain-rej'; +export {assume} from './assume'; export {and} from './and'; export {both} from './both'; diff --git a/src/future.mjs b/src/future.mjs index eb18cca3..7c1f0f11 100644 --- a/src/future.mjs +++ b/src/future.mjs @@ -616,6 +616,11 @@ defineBimapperAction('fold', { rejected: function FoldAction$rejected(x){ return mapWith(this.lmapper, resolve, x) } }); +defineOtherAction('assume', { + rejected: returnOther, + resolved: returnOther +}); + defineOtherAction('and', { resolved: returnOther }); diff --git a/test.js b/test.js index 4e7febad..fd2a856c 100644 --- a/test.js +++ b/test.js @@ -21,6 +21,7 @@ import './test/5.chain.test.mjs'; import './test/5.encase-n.test.mjs'; import './test/5.encase-p.test.mjs'; import './test/5.encase.test.mjs'; +import './test/5.assume.test.mjs'; import './test/5.fold.test.mjs'; import './test/5.hook.test.mjs'; import './test/5.map-rej.test.mjs'; diff --git a/test/2.transformation.test.mjs b/test/2.transformation.test.mjs index df0bb8e1..4e69c1dc 100644 --- a/test/2.transformation.test.mjs +++ b/test/2.transformation.test.mjs @@ -340,6 +340,34 @@ describe('Transformation', function (){ }); + describe('assume', function (){ + + var seq = dummy.assume(dummy); + + describe('#_interpret()', function (){ + + it('runs the action', function (){ + return assertResolved(seq, 'resolved'); + }); + + it('runs the other if the left rejects', function (done){ + var other = Future(function (){done()}); + var m = new Transformation(rejected, nil).assume(other); + m._interpret(done, noop, noop); + }); + + }); + + describe('#toString()', function (){ + + it('returns code to create the same data-structure', function (){ + expect(seq.toString()).to.equal('Future.of("resolved").assume(Future.of("resolved"))'); + }); + + }); + + }); + describe('in general', function (){ describe('#_interpret()', function (){ diff --git a/test/5.assume.test.mjs b/test/5.assume.test.mjs new file mode 100644 index 00000000..f35f0a80 --- /dev/null +++ b/test/5.assume.test.mjs @@ -0,0 +1,109 @@ +import chai from 'chai'; +import {Future, assume, of, reject} from '../index.mjs'; +import * as U from './util'; +import * as F from './futures'; +import type from 'sanctuary-type-identifiers'; + +var expect = chai.expect; + +var testInstance = function (assume){ + + it('is considered a member of fluture/Fluture', function (){ + expect(type(assume(of(1), of(2)))).to.equal(Future['@@type']); + }); + + describe('#_interpret()', function (){ + + it('runs the second Future when the first resolves', function (done){ + assume(of(1), of(null).map(done))._interpret(done, U.noop, U.noop); + }); + + it('runs the second Future when the first rejects', function (done){ + assume(reject(1), of(null).map(done))._interpret(done, U.noop, U.noop); + }); + + it('always rejects with the rejection reason of the second', function (){ + var actualResolved = assume(of(1), reject(2)); + var actualRejected = assume(reject(1), reject(2)); + return Promise.all([ + U.assertRejected(actualResolved, 2), + U.assertRejected(actualRejected, 2) + ]); + }); + + it('always resolves with the resolution value of the second', function (){ + var actualResolved = assume(of(1), of(2)); + var actualRejected = assume(reject(1), of(2)); + return Promise.all([ + U.assertResolved(actualResolved, 2), + U.assertResolved(actualRejected, 2) + ]); + }); + + it('does nothing after being cancelled', function (done){ + assume(F.resolvedSlow, F.resolved)._interpret(done, U.failRej, U.failRes)(); + assume(F.resolved, F.resolvedSlow)._interpret(done, U.failRej, U.failRes)(); + assume(F.rejectedSlow, F.rejected)._interpret(done, U.failRej, U.failRes)(); + assume(F.rejected, F.rejectedSlow)._interpret(done, U.failRej, U.failRes)(); + setTimeout(done, 25); + }); + + }); + +}; + +describe('assume()', function (){ + + it('is a curried binary function', function (){ + expect(assume).to.be.a('function'); + expect(assume.length).to.equal(2); + expect(assume(of(1))).to.be.a('function'); + }); + + it('throws when not given a Future as first argument', function (){ + var f = function (){ return assume(1) }; + expect(f).to.throw(TypeError, /Future.*first/); + }); + + it('throws when not given a Future as second argument', function (){ + var f = function (){ return assume(of(1), 1) }; + expect(f).to.throw(TypeError, /Future.*second/); + }); + + testInstance(function (a, b){ return assume(b, a) }); + +}); + +describe('Future#assume()', function (){ + + it('throws when invoked out of context', function (){ + var f = function (){ return of(1).assume.call(null, of(1)) }; + expect(f).to.throw(TypeError, /Future/); + }); + + it('throws TypeError when not given a Future', function (){ + var xs = [NaN, {}, [], 1, 'a', new Date, undefined, null, function (x){ return x }]; + var fs = xs.map(function (x){ return function (){ return of(1).assume(x) } }); + fs.forEach(function (f){ return expect(f).to.throw(TypeError, /Future/) }); + }); + + testInstance(function (a, b){ return a.assume(b) }); + +}); + +describe('Future#assume()', function (){ + + it('throws when invoked out of context', function (){ + var f = function (){ return of(1).assume.call(null, of(1)) }; + expect(f).to.throw(TypeError, /Future/); + }); + + it('throws TypeError when not given a Future', function (){ + var xs = [NaN, {}, [], 1, 'a', new Date, undefined, null, function (x){ return x }]; + var fs = xs.map(function (x){ return function (){ return of(1).assume(x) } }); + fs.forEach(function (f){ return expect(f).to.throw(TypeError, /Future/) }); + }); + + testInstance(function (a, b){ return a.assume(b) }); + +});