useUrl hook for router #338
Conversation
There was a problem hiding this comment.
Requesting for use0 and just making sure we have a plan here. Code looks good (and super excited for this).
cc @chenglou since he's invested in this router.
src/ReasonReact.re
Outdated
| let watcherId = watchUrl(url => setUrl(_ => url)); | ||
| Some(() => unwatchUrl(watcherId)); | ||
| }, | ||
| [||], |
There was a problem hiding this comment.
use(Layout)Effect0 should get you this effect already.
There was a problem hiding this comment.
Ah, right! That's convenient!
| let useUrl = () => { | ||
| let (url, setUrl) = React.useState(dangerouslyGetInitialUrl); | ||
|
|
||
| React.useLayoutEffect0(() => { |
There was a problem hiding this comment.
Why did you opt for LayoutEffect here? I think that because this doesn’t need to be synchronous Effect might be a better choice, but I’m not sold either way really. I don’t think this is blocking this PR but since it would technically be a breaking change I think we should resolve before a wider release.
There was a problem hiding this comment.
The use case I had was only working with useLayoutEffect.
(In my app, for some routes, I would render a Redirect component that would push a new route. For some reason, this only worked when using useLayoutEffect here and useEffect in the Redirect component. I did not investigate it any further.)
There was a problem hiding this comment.
Interesting. LayoutEffect happens earlier than Effect so perhaps these hooks are called upward and this order is required in order to trigger this subscription before the Redirect component executes. That seems like a reasonable excuse to put this here.
No description provided.