purescript-react / purescript-react-basic

An opinionated set of bindings to the React library, optimizing for the most basic use cases
https://pursuit.purescript.org/packages/purescript-react-basic
Apache License 2.0
281 stars 40 forks source link

Add roles declarations to allow safe coercions #128

Open kl0tl opened 4 years ago

megamaddu commented 4 years ago

Do you have any info on the what/why here? I also need to update this branch with the recent changes from main, specifically moving some of these types to https://github.com/lumihq/purescript-react-basic-classic

hdgarrood commented 4 years ago

The context is that foreign import data declarations are always given nominal roles by default, which means that without these declarations you won't be able to e.g. coerce a ReactComponent { x :: Int } to a ReactComponent { x :: Additive Int }. These changes aren't absolutely necessary to have this library work with 0.14.0, they just allow you to do coercions where you otherwise wouldn't be able to. Note that this is a breaking change, as we'd be dropping support for 0.13.x and earlier.

megamaddu commented 4 years ago

Ah, I see. Seems we'd probably want to delay it a bit unless there are other parts of the 0.14 PR that aren't backwards compatible already.

kl0tl commented 4 years ago

There’s already backward incompatible changes on the ps-0.14 branch (the RowList kind gained a parameter with the advent of polykinds) so I thought we could merge this on ps-0.14 and then merge ps-0.14 on main when a v014.0 of the compiler is released.

kl0tl commented 4 years ago

Also I‘ve a branch ready for purescript-react-basic-classic but didn’t open a pull request yet because the repository has no v0.14.0 compatible branch. I‘m trying to coordinate this on Discourse, see https://discourse.purescript.org/t/roles-declarations-to-add-for-v0-14-0-package-set.

megamaddu commented 4 years ago

Ok. The other 0.14 changes should all be here in this branch, they just need to be copied out to the other repos. (-dom, -classic, -compat)