Require a return value for all transformative functions#190
Require a return value for all transformative functions#190jleider wants to merge 3 commits intomonet:developfrom
Conversation
…s included with strict
This enforces that calls to map, leftMap, cata, etc. provide a return value where it is expected.
ulfryk
left a comment
ulfryk
left a comment
There was a problem hiding this comment.
Thanc
ks for contribution!
- We shouldn't enforce TS higher than 3.0.x in
0.9.0so this PR wil wait till post release - This should also include all
flatMaps,chains,binds andcatchMaps
👍
| "karma-jasmine": "1.1.2", | ||
| "karma-mocha-reporter": "2.2.5", | ||
| "typescript": "3.0.3", | ||
| "typescript": "3.1.3", |
There was a problem hiding this comment.
it would be nice to not enforce upgrade to TS higher than 3.0
| // uncomment to fail typecheck for non return value | ||
| // twelve.map(() => { "no return"; }); | ||
| // twelve.leftMap(() => { "no return"; }); | ||
| // twelve.cata(() => { "no return" }, () => { "no return"; }); |
There was a problem hiding this comment.
cata should allow to return void
Use of I'd just not allow chaining - call to side effects should e the last thing done ;) Optionally we may introduce WDYT @jleider ? |
|
@jleider - is there a chance you'd continue on this? |
|
@ulfryk Unfortunately not, we migrated to https://gcanti.github.io/fp-ts/ even though they don't provide a |
2 participants
This enforces that calls to
map,leftMap,cata, etc. provide a return value where it is expected.One caveat to note is that users will find that side-effectual code will now throw type errors. For example, we use
cata/foldfairly extensively in our codebase. Some of the cases we had side-effectual functions that didn't have a return value. These now need to be wrapped inIO. Additionally, there could be a convenience function that would operate on both left/right, some/none, etc. likecatabut for side-effects similar toforEachsince currently there is no way to chainforEachandorElseRunbecause they both returnvoidinstead ofthis.Requires TypeScript 2.8+.