Skip to content

Conversation

@emmenko
Copy link
Contributor

@emmenko emmenko commented Jan 6, 2018

Small improvements. I left out some other improvements which are covered by the #6 PR.

In details:

  • use in-source mode to generate *.bs.js files next to the source
  • compiled sources are in es6
  • upgraded some deps versions
    • added bs-platform as a peerDependency
  • be more specific about the match object shape (params)
  • added bindings for Redirect component
  • removed unnecessary withROuter function

@chenglou chenglou merged commit 3245c83 into reasonml-old:master Jan 10, 2018
@chenglou
Copy link
Contributor

Thanks! Btw you might have seen the new https://reasonml.github.io/reason-react/docs/en/router.html. Give it a try! I'm moving this repo to reasonml-old for now.

@emmenko
Copy link
Contributor Author

emmenko commented Jan 10, 2018

Just saw that yes, looks pretty neat with pattern matching! 🙌

Btw: isn’t the notice in the readme enough? People with existing projects would still be using the bindings I suppose 🤔
So I wouldn’t throw this library away for now...

@chenglou
Copy link
Contributor

chenglou commented Jan 10, 2018

We're not deleting the library for sure! Just that keeping it inside reasonml-community is slightly of a burden (for me. I use a multi-codebases git workflow). The new RR router also interops with your existing code, so I think converting react router part to RR router might not be that much more work vs converting to bs-react-router first.

But if someone wants to handle it by moving to his/her own repo, that's nice too.

@emmenko
Copy link
Contributor Author

emmenko commented Jan 10, 2018

Yeah I understand. I guess I’ll have to try it out first to see how much effort it is (at least for me) 😄
And thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants