Refactor to prepare for release #14
Conversation
README.md
Outdated
| @@ -1,30 +1,9 @@ | |||
| # purescript-web-router | |||
|
|
|||
| A basic pushstate router for React with support for asynchronous routing logic, built using [react-basic-hooks](https://github.com/spicydonuts/purescript-react-basic-hooks). I recommend [routing-duplex](https://github.com/natefaubion/purescript-routing-duplex) for easy parsing and printing. | |||
| A basic pushstate router for React with support for asynchronous routing logic. I recommend [routing-duplex](https://github.com/natefaubion/purescript-routing-duplex) for easy parsing and printing. | |||
There was a problem hiding this comment.
| A basic pushstate router for React with support for asynchronous routing logic. I recommend [routing-duplex](https://github.com/natefaubion/purescript-routing-duplex) for easy parsing and printing. | |
| A basic pushstate router with support for asynchronous routing logic. I recommend [routing-duplex](https://github.com/natefaubion/purescript-routing-duplex) for easy parsing and printing. |
There was a problem hiding this comment.
The readme looks... a little different now 🙂
There was a problem hiding this comment.
README looks great :-)
Do you think that making a README part of a "test suite" (so we verify whether it still compiles) is feasible and desired?
I mean I use this strategy across my libs just by using something like:
https://github.com/paluh/purescript-homogeneous/blob/master/package.json#L10 (litps is really a tool written by @Thimoteus).
There was a problem hiding this comment.
If PS supported literate code I'd consider it, but I don't think it's worth the extra effort. The example project using React has also been updated and it compiles, so most of the code in the updated readme was tested there first.
Some unit tests would be good, but can come a bit later.
There was a problem hiding this comment.
This litps provides support for "literate PS". Maybe I will play around that idea in my... spare time ;-)
|
|
||
| let overrides = {=} | ||
|
|
||
| let additions = {=} |
There was a problem hiding this comment.
It would be nice to drop these spago comments all together because they clutter the actual diffs :-P
There was a problem hiding this comment.
That's a fair point. I've stripped them out.
There was a problem hiding this comment.
Hi Robert!
Sorry for jumping ahead and commenting your PR but I want to say that I really like these changes. From a user perspective I can say that:
RouterMandRoutingis much cleaner naming convention thanTransitionandTransitioning- The same goes for
RouterStatevsRouterEvent - As a consequence the surface API of
mkRouteris now beautiful and easy to understand :-)
Thanks a lot for this update!
|
LGTM 👍 |
This is a long-overdue clean up of parts of the routing logic and underlying implementation.
@jonasbuntinx Care to give this a look-over?
The downstream changes are pretty mild: https://github.com/robertdp/purescript-web-router-example/pull/2/files#diff-eb73bc89dc00e8262fee8eaa74c28d14d54c2d71c3275faddd28870baa9370ea