-
Notifications
You must be signed in to change notification settings - Fork 240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds onEnter function to UserAuthWrapper #19
Conversation
@themouette this seems to work pretty well for onEnter, but Im not really happy with how it uses the query parameter on for the auth check but the history state for onEnter. Seems like this would be pretty annoying to have to check the state or query parameter depending on if the onEnter or Higher Order Component redirected. Thoughts? |
That's great! Thanks a lot.
Indeed, maybe using the query parameter for both browser and server is the best strategy as it is available everywhere. |
@@ -106,6 +106,14 @@ const UserAuthWrapper = (args) => { | |||
|
|||
return hoistStatics(UserAuthWrapper, DecoratedComponent) | |||
} | |||
|
|||
wrapComponent.onEnter = (store, nextState, replace) => { | |||
const replaceWithState = ({ pathname, query }) => replace({ pathname, state: query }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I read on the react-router doc, to use query string instead of state would be done by replacing previous line with:
const replaceWithState = ({ pathname, query }) => replace({ pathname, query })
Yeah I started with using the query key, but the tests on travis seemed to be failing to validate that the query got set (worked locally). Going to switch back to the query key and do a little more debugging. There's nothing from the React-Router docs that would expect it to not work that way. |
well that works now, maybe the RR bump helped or just was a temporary travis issue. |
Adds onEnter function to UserAuthWrapper
Useful in server side rendering #17