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

Child route has been remounted #58

Closed
fezhengjin opened this issue Jul 27, 2016 · 12 comments
Closed

Child route has been remounted #58

fezhengjin opened this issue Jul 27, 2016 · 12 comments
Labels

Comments

@fezhengjin
Copy link

fezhengjin commented Jul 27, 2016

Hi @mjrussell, this is my routes config.

// Redirect to '/setup' if cluster has not been installed.
const ClusterIsInstalled = UserAuthWrapper({
  authSelector: state => state.installationMap,
  authenticatingSelector: state => state.installationMap.get('isFetching'),
  predicate: installationMap => installationMap.get('finished'),
  failureRedirectPath: '/setup',
  allowRedirectBack: false,
});

// Redirect to '/login' if user has not been logged in.
const UserIsAuthenticated = UserAuthWrapper({
  authSelector: state => state.authMap,
  predicate: authMap => authMap.get('isAuthenticated'),
  wrapperDisplayName: 'UserIsAuthenticated',
});

export default (
  <Route path="/" component={App}>
    <Route path="/setup" component={ClusterIsNotInstalled(SetupPage)} />
    <Route path="/login" component={UserIsNotAuthenticated(ClusterIsInstalled(LoginPage))} />
    <Route path="/dashboard" component={UserIsAuthenticated(ClusterIsInstalled(Dashboard))} >
      <IndexRoute component={OverviewPage} />
      <Route path=":resource" component={ResourcePage} />
    </Route>
  </Route>
);

When the browser's url match the resource page route such as "/dashboard/osds", the "ResourcePage" component has been mounted twice. And the second one was occurred after the "GET_CLUSTER_INSTALLATION_SUCCESS" redux action.

This is my console log:

Navigated to http://localhost:3000/dashboard/osds
ResourcePage WillMount          ResourcePage.js?56ee:89
[HMR] connected                 client.js?3ac5:55
ResourcePage WillUnmount        ResourcePage.js?56ee:90
Dashboard WillMount             Dashboard.js?9bc2:55
ResourcePage WillMount          ResourcePage.js?56ee:89

What can i do if i don't want the "ResourcePage" be remount?

@andrewmclagan
Copy link

yeah same issue here

@mjrussell
Copy link
Owner

So if you are navigating to a subroute, even when you are authenticated, the parent mounts and then unmounts? Is it first sending you to the login page? I added some logging locally to the basic example with a nested route example and I couldn't reproduce this. Do you have a small example I could look at?

The routes you included don't have this issue, but I have seen some weird mounting/unmounting behavior when people use dynamic routing (i.e. getComponent(s)) because they apply the HOC in the function and it returns technically a new Component class each time.

@mjrussell mjrussell added the bug label Jul 28, 2016
@fezhengjin
Copy link
Author

Hi @mjrussell , sorry for the late reply.

To reproduce this, i forked a standalone project and commit some change.

Then, if you are navigating to the "ProtectedSubView" subroute by clicking the "Attempt to access the protected sub view" link in the HomeView, you'll reproduce this problem.

action @ 12:02:53.249 FETCH_PROTECTED_DATA_REQUEST 
sub view page did mount ProtectedSubView.js:6
sub view page will unmount ProtectedSubView.js:9
sub view page did mount ProtectedSubView.js:6
action @ 12:02:53.684 RECEIVE_PROTECTED_DATA index.js:152 

@mjrussell
Copy link
Owner

Thank you for the example, that was a good start.

I got to the bottom of this, it is related to how the authenticatingSelector works with the default LoadingComponent. Because you are passing the authenticatingSelector as a property, it will render the span which is the default LoadingComponent passing through all props from the parent of the wrapper component. One of these props is children from React Router so the result while loading is essentially the JSX of:

<span>
  <ResourcePage />
</span>

and then once authenticated:

<ResourcePage />

Obviously I think that is unintuitive for most users, but I would also say that unless you specifically want to display some Component during loading (i.e. logging in) you don't need the authenticatingSelector at all.

@andrewmclagan
Copy link

andrewmclagan commented Aug 2, 2016

ah i see! was having the same issue.... yeah i feel that is super unintuitive.

@fezhengjin
Copy link
Author

fezhengjin commented Aug 2, 2016

This resolve my problem, thank you!
I specify a LoadingComponent which not render the children property, then the subroute will not be remounted.

@mjrussell
Copy link
Owner

Yeah Im going to change the default loading component to drop all props passed into it and release a new version shortly. It will complain loading in react 15.2 as is also due to unknown props

@peteruithoven
Copy link

but I would also say that unless you specifically want to display some Component during loading (i.e. logging in) you don't need the authenticatingSelector at all.

Not necessarily true, on startup I have to check whether my login session is still valid, which is a async check. I now use the authenticatingSelector to handle this.

@mjrussell
Copy link
Owner

mjrussell commented Aug 4, 2016

@peteruithoven can you explain a little more about what you are doing? You shouldn't be performing any side effects inside the authenticating selector. The authors of react-redux have said that performing a side effect in their mapStateToProps or mapDispatchToProps (both selectors are used in mapStateToProps) is unsupported and can break at any time - http://stackoverflow.com/questions/36654197/should-mapdispatchtoprops-dispatch-initialization-actions/36655872#36655872.

@peteruithoven
Copy link

I'm not, the selectors just check the state. When starting there is a async check whether the session is still valid. While doing this there is a isPending boolean in the state, that my authenticating selector checks. My isAuthenticated simply checks whether there is session data, but it doesn't know whether it's checked. So by combining the 2 selectors I get what I want.
Another way to do this would be to only store the session in the state when checked, or having a predicate check both the session and the isPending state. But both where more work.

@mjrussell
Copy link
Owner

Oops you are right, I wasn't thinking correctly for a minute there. Yup makes sense and that was the original intent of the isAuthenticating

@mjrussell
Copy link
Owner

Released 0.7.0 which fixes this

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

No branches or pull requests

4 participants