Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.0.0 #312

Merged
merged 135 commits into from
Feb 12, 2018
Merged

1.0.0 #312

merged 135 commits into from
Feb 12, 2018

Conversation

gcanti
Copy link
Owner

@gcanti gcanti commented Feb 6, 2018

@raveclassic @sledorze

Changelog: see commits

Try: npm i gcanti/fp-ts#next

@gcanti
Copy link
Owner Author

gcanti commented Feb 7, 2018

Ok, published on npm

  • fp-ts@next (fp-ts@1.0.0-rc)
  • io-ts@next (io-ts@0.9.8-rc)
  • monocle-ts@next (monocle-ts@0.5.3-rc)
  • io-ts-types@next (io-ts-types@0.2.5-rc)
  • newtype-ts@next (newtype-ts@0.1.3-rc)

Except fp-ts, the other packages should NOT have API breaking changes

Copy link
Collaborator

@raveclassic raveclassic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should 1.0-rc briefly describe the breaking changes in readme?

@gcanti
Copy link
Owner Author

gcanti commented Feb 7, 2018

Those releases are mainly for early feedback, I'm waiting to update the documentation when things settle down and we are satisfied by the result.

Namely

  • changelog (will be huge)
  • docs/HKT.md

Conversely examples and exercises should be up to date

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti Here I will consolidate the issues I encounter:

Problem with Either predicates.

Previous version:

        import { either } from 'fp-ts'
...
        arrayOfEithers
        .filter(either.isRight) // predicate
        .map(x => ...) // x is Right<..> :)

New version:

        arrayOfEithers
        .filter(x => x.isRight()) // predicate refines x 
        .map(x => ...) // x is Either<..> :(

We're missing a top level support:

export const isRight = <A, B>(x: either.Either<A, B>): x is either.Right<A, B> =>
  x.isRight()

Note that the same reasoning applies to other Sum types.. (Option, These, ..)

@gcanti
Copy link
Owner Author

gcanti commented Feb 9, 2018

@sledorze I would argue that using Right as a type is wrong: Right is a constructor not a type (well in TypeScript is also a type because of the implementation and because I'm forced to export Left and Right by TypeScript itself but should be considered as an implementation detail)

In PureScript for example you can't use Right as a type

data Either l r = Left l | Right r

x :: Right String -- doesn't compile
x = Right "foo"

x :: Either Number String -- ok
x = Right "foo"

p.s.

Also in your particular case there's Array.rights

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti this is not on an array but on (rxjs) observables the use case is not easy without those refinements (sorry for having over simplified the example).
The only solution is to recreate that top level predicate (idiomatic) OR to define another Observable combinator (which a end user would see as the least solution).
Also Right, as implemented, is a type in typescript and its usage is very useful and idiomatic; why restricting it to the possibilities of Purescript.
That's why I think we should (re)add it (based on a real world code base).

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti
Note on the order of params.
I see that Functor and Chain params are now inverted like so:

chain: <A, B>(fa: HKT<F, A>, f: (a: A) => HKT<F, B>) => HKT<F, B>;
map: <A, B>(fa: HKT<F, A>, f: (a: A) => B) => HKT<F, B>;

But on Apply that's not the case:

ap: <A, B>(fab: HKT<F, (a: A) => B>, fa: HKT<F, A>) => HKT<F, B>;

is it deliberate? (if so, why?)

@gcanti
Copy link
Owner Author

gcanti commented Feb 9, 2018

why restricting it to the possibilities of Purescript

PureScript is my reference when it comes to establish what's good and what's bad, in particular with respect to functional programming.

However if there is consensus I'm ok to re-add those custom guards because

  • clearly you find them useful
  • one less breaking change

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

Ok anyway i m adding the missing bits into a file so that we can review it later.

@gcanti
Copy link
Owner Author

gcanti commented Feb 9, 2018

is it deliberate? (if so, why?)

Based on my early experiments there's no gain in terms of type inference / suggestions by swapping the Apply arguments. I'll investgate further though

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti i was specifically thinking about approachability by homogeneity (if not, wondering what s the default/preferred behavior)

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti I find we lack meaningful definition for Ordering (I miss LT, EQ, GT) even if they map to -1, 0, 1 in the end.

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti Ok so I've ported our code base on the new version.

Notable:

  • A lot of foldL usage (63).

  • new order of args leads to better inference AND looks better also with prettier; often we can see everything in the first line as the biggest param (the function) is the last one.

  • Took me a bit of time to understand there was new Monad1, Monad2, etc.. while porting some monadTransformers.

  • miss the isRight / isSome a lot in the front-end side (observables) by our frontEnd guy; for that kind of two stage usages: opt$.filter(isSome).map(x => x.value).

  • no array.concat; I'm using array.flatten, I don't know if it's better or not performance wise.

  • Less type params to write explicitly.

Overall, I would consider this version as ready for usage.

@sledorze
Copy link
Collaborator

sledorze commented Feb 9, 2018

@gcanti oh I forgot to say that io-ts-types are not referencing the new libs explicitly.
That prevents me to share the new repo internally.

@gcanti
Copy link
Owner Author

gcanti commented Feb 10, 2018

@sledorze great feedback, thanks

miss the isRight / isSome

ok I'm going to re-add them (*)

no array.concat

you can import concat either from the function module or calling Array.getSemigroup()

io-ts-types are not referencing the new libs explicitly

My plan would be

  • release fp-ts@1.0.0 (after (*))
  • merge 1.0.0 io-ts#112 and release io-ts@1.0.0
  • update all remaining libraries (inluded a cleaned up io-ts-types)

@gcanti gcanti changed the title 1.0.0-rc 1.0.0 Feb 10, 2018
@gcanti
Copy link
Owner Author

gcanti commented Feb 11, 2018

UPDATE

I'm working on a bunch of PRs, the goal would be to coordinate all releases in one shot. Here's the dependencies

fp-ts (v1.0.0) -- io-ts (v1.0.0)
              \-- monocle-ts (v1.0.0)
                                    \-- newtype-ts (v0.2.0)
                                                           \-- io-ts-types (v0.3.0)

You can install the current candidates by running

  • npm i gcanti/fp-ts#next
  • npm i gcanti/io-ts#next (related PR here)
  • npm i gcanti/monocle-ts#next (related PR here)
  • npm i gcanti/newtype-ts#next (related PR here)
  • npm i gcanti/io-ts-types#next (related PR here)

(for these kind of tests / quick feedback installing from github looks more practical than publishing on npm)

Let me know if you hit any issue

@sledorze
Copy link
Collaborator

Ok, got no issue so far (but was not able to test with your latest changes - aka type guards - as they've not been published yet).

@gcanti
Copy link
Owner Author

gcanti commented Feb 11, 2018

as they've not been published yet

@sledorze make sure to install fp-ts by running npm i gcanti/fp-ts#next (<= i.e. from GitHub not from npm)

@gcanti gcanti merged commit 8ddf3b8 into master Feb 12, 2018
@gcanti gcanti deleted the 1.0.0 branch February 13, 2018 10:30
@gcanti gcanti removed the WIP label Feb 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants