[DO NOT MERGE] Switch to @reach/router

c8r / x0

Document & develop React components without breaking a sweat

https://compositor.io/x0/

MIT License

1.71k stars 65 forks source link

[DO NOT MERGE] Switch to @reach/router #90

Open ryanflorence opened 6 years ago

ryanflorence commented 6 years ago

Hey folks 👋🏼

I talked to @jxnblk and he expressed interest in switching to Reach Router (https://reach.tech/router). A few reasons to do it:

route transitions are more accessible (https://reach.tech/router/accessibility)
bundle sizes are smaller
relative links are really convenient

I was surprised how straightforward the migration was, only took an hour or so 😮

A couple todos:

[ ] get npm run build to work, (I couldn't get it to work on master either, something about the xo/ basename, so I think it's fine? need somebody else to confirm)
[ ] update documentation that references Link or any other router API

What do you think?

jxnblk commented 6 years ago

This looks great, thanks!

get npm run build to work, (I couldn't get it to work on master either, something about the xo/ basename, so I think it's fine? need somebody else to confirm)

By get it to work, do you mean running the exported site in a local server? The basename is set up so that it works at https://compositor/x0 and it's fine to disable that flag for testing it out locally...

update documentation that references Link or any other router API

Yeah, I think it might be nice to hide the internals of routing a bit better, e.g. have a Link component that can be imported from this library. I would guess that switching out the router implementation here will be a major version change

ryanflorence commented 6 years ago

The basename is set up so that it works at https://compositor/x0 and it's fine to disable that flag for testing it out locally

Yes, I got it to run by removing pkg.x0.basename, but I wanted to test out the basename handling too. I don't know how to get the basename in at build time on master or this branch.

ryanflorence commented 6 years ago

Yeah, I think it might be nice to hide the internals of routing a bit better,

If you'd like. I don't have an opinion either way. Tradeoff seems to be that now you have an API you have to maintain (so if somebody wants to render some router aware <Tabs/> things aren't as straightforward, you'll have to document it, add more code, make more decisions about what part of the API you copy, what you change, what you leave out, etc.), but upside is if you switch routers again it wouldn't be a breaking change.