Remove cancellation hook from Future.finally #281
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #281 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 48 48
Lines 990 989 -1
Branches 215 215
=====================================
- Hits 990 989 -1
Continue to review full report at Codecov.
|
To make Future.finally more reliable as a tool which guarantees execution of the cleanup handler, it would run and cancel the handler when it would itself be cancelled. However, this behaviour is subject to some issues: - The cleanup handler would run when a Future prior to the finally-call got cancelled, leading to attempts to dispose yet-to-be-acquired resources. - The cleanup handler would be immediately cancelled, which meant that unless it was prepared for ignoring the cancellation signal, cleanup wouldn't even happen. This commit fixes both of those issues by removing this behaviour altogether. Finally then becomes a function to use solely for cases where something should happen as a side-effect under rejection and under resolution of a Future, as its documentation already states.
Avaq
added a commit
that referenced
this pull request
Aug 30, 2018
Breaking changes - #276 The "module" package file has been renamed from index.mjs.js to index.mjs. - #277 It is no longer possible to use Future.ap, Future.map, Future.bimap, Future.chain, or Future.alt on non-Fantasy Land types. If you were using any of these functions from Fluture to treat JavaScript's native types as Fantasy Land algebraic types, you should migrate those call-sites to use the Ramda or Sanctuary exported dispatchers instead. - #281 Future.finally no longer runs the cleanup Future when it is cancelled. If you rely on this behaviour, I urge you to rewrite the relevant code with Future.hook (even if you don't upgrade Fluture). New features - #276 The Fluture source is now loadable by Node's experimental module loader, and the esm loader (https://github.com/standard-things/esm). - #280 The Future.of function now has an alias 'resolve', which is exported statically as well as exposed as a property on the Future constructor. Bug fixes and improvements - #277 Fluture no longer depends on sanctuary-type-classes, reducing bundle size. - #277 Performance of Fantasy Land dispatchers greatly improved. - #281 The undesired consequences of the cancellation-related behaviour in Fluture.finally are gone now that the behaviour has been removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A less severe alternative to #279
To make Future.finally more reliable as a tool which guarantees
execution of the cleanup handler, it would run and cancel the handler
when it would itself be cancelled.
However, this behaviour is subject to some issues:
got cancelled, leading to attempts to dispose yet-to-be-acquired
resources.
unless it was prepared for ignoring the cancellation signal, cleanup
wouldn't even happen.
This commit fixes both of those issues by removing this behaviour
altogether. Finally then becomes a function to use solely for cases
where something should happen as a side-effect under rejection and under
resolution of a Future, as its documentation already states.