search

ethul / purescript-react-redux

MIT License
15 stars 6 forks source link

Simplify #10

Closed ethul closed 7 years ago

ethul commented 7 years ago

I've been experimenting with some breaking changes in this PR.

I am considering merging this in soon, but I definitely open to any feedback you might have since you've all contributed to the repository. Thanks!

@coot @pbrant @felixSchl

PS - Updates to the example https://github.com/ethul/purescript-react-redux-example/pull/2

felixSchl commented 7 years ago

No objections here, looks good to me.

ethul commented 7 years ago

Thanks for all of your feedback! I want to make a few more small changes, but should get this out in a release soon.

pbrant commented 7 years ago

A little late to the party, but I like this too. Ditto on the move to newtypes. Thanks for continuing to experiment with this.

ethul commented 7 years ago

Thanks for taking a look @pbrant!

ethul commented 7 years ago

@BartAdv you had some good ideas in #6. I am wondering if you had any thoughts on the proposed changes here. Any feedback is definitely appreciated!

ethul commented 7 years ago

Just to note, release of these changes depends on purescript/purescript-record#7

BartAdv commented 7 years ago

@ethul quite a lot of changes! This means, unfortunately, I won't be able to dig into this stuff, as I'm not using this library at this moment.

felixSchl commented 7 years ago

Will this be possible with this simplification? Or should I create a separate issue? Essentially I want to pass the { withRef: true} option to connect

ethul commented 7 years ago

@felixSchl Good call! No need to raise an issue. I will look into incorporating that into this PR. Thanks!

ethul commented 7 years ago

@BartAdv Indeed! Revamping a lot in this PR. Thanks for the update!