acdlite / redux-router

Redux bindings for React Router – keep your router state inside your Redux store
MIT License
2.3k stars 216 forks source link

rerender on route change #214

Closed mmahalwy closed 8 years ago

mmahalwy commented 8 years ago

is it just me or the previous route's component rerenders right before a transition to the new one due to params changing and react-redux passing through props the new props to the old route.

I found the only way to get around this is to implement my own shouldComponentUpdate. Any suggestions or anyone found same challenge?

mmahalwy commented 8 years ago

Also, I have noticed that either this or react-router is not full unmounting the previous route.

apapirovski commented 8 years ago

@mmahalwy That is correct. connect-ed components will propagate state changes first before a render with the new component tree happens. Unfortunately, there's no good solution to it the way redux-router is implemented.

To be honest, as someone using this in production, I think for full compatibility with redux, there needs to be a router that's completely independent of react-router but dependent on history. Otherwise you have to architect your whole app (and its stores) to work around these type of incompatibilities.

mmahalwy commented 8 years ago

@apapirovski so i found a really good solution.Your routes will have params passed to it from react-router, which doesnt change. That is what should be used. I honestly don't have a current use case for redux-router at all other than the redux devtools and going back in time, and other redux cool stuff. For your curiosity here is my implementation:

@connect(
  state => {
    debug('Component:Region:@connect', '@connect');
    if (state.regions.errored) {
      return {
        errored: true
      };
    }

    return {
      regions: state.regions.data,
      regionsMap: state.regions.map,
      regionImages: state.regionImages,
      tagPages: state.tagPages,
      images: state.images.data,
      activities: state.activities.data,
      tags: state.tags.data,
      params: state.router.params
    };
  },
  null,
  (stateProps, dispatchProps, ownProps) => {
    const { params } = ownProps;
    let id = params.regionId;

    if (!isJeevesId(params.regionId)) {
      id = stateProps.regionsMap[params.regionId];
    }

    const region = stateProps.regions[id];
    const tagPages = region.tagPageIds.map(tagPageId => stateProps.tagPages.data[tagPageId]).filter(tp => !!tp);
    const regionImages = region.regionImageIds.map(imageId => stateProps.regionImages.data[imageId]);

    return {
      ...stateProps, ...dispatchProps, ...ownProps,
      region,
      tagPages,
      regionImages
    };
  }
)
apapirovski commented 8 years ago

@mmahalwy Yes (and that's what we do on our project) but it's very a non-redux way of handling things. There are still issues, if for example, you use async fetching of data where your connected components will update with new state before a transition happens.

Mafioso commented 8 years ago

Please refer also to https://github.com/acdlite/redux-router/issues/219. I think that's an issue. If it propogates state changes on a previous component but with different state, redux-router does not guarantee idempotancy, which means it does not guarantee a pure state at all.