Skip to content
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

Optional redirectPath? #200

Open
alvelig opened this issue Sep 9, 2017 · 1 comment
Open

Optional redirectPath? #200

alvelig opened this issue Sep 9, 2017 · 1 comment

Comments

@alvelig
Copy link

alvelig commented Sep 9, 2017

connectedReduxRedirect({
  //redirectPath: "/login", <--- should not be required as it's overridden by redirectAction
  redirectAction: (currentLocation) => ({
    type: "AUTHENTICATION_REQUIRED",
    payload: currentLocation
  }),
  authenticatedSelector,
})

Should redirectPath option in connectedReduxRedirect be optional when redirectAction is used?

That would be handy in ReactNative using ReactNavigation with Redux integration, or other no-location redux-based navigators, or implementing redux-saga navigation control for example.

Or if we use router, but rely on redux, then if redirectPath is undefined, pass current router location, thus allowing to dispatch action with payload containing current location.

@mjrussell
Copy link
Owner

Hmm this is interesting. Right now the redirectPath is necessary for the connected versions because its used to build the new location. Its not the "currentLocation" but rather the location to redirect to. I could see a use for no-location based navigators, but I think that should be its own helper HOC, built on top of the same helpers as opposed to tweaking one that might make it more confusing for the majority of users. What parameters would you want in such an HOC? I could also see just passing the old location to the redirectAction in case you want to dispatch an action based on it. That seems like an easy improvement that doesn't break anything.

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

No branches or pull requests

2 participants