redux-auth-wrapper: Causes all subcomponents to re-render on any state change

I recently noticed that every component in my app was re-rendering with every top-level state change, even those totally unaffected. I use react-router-4, redux, reselect and Immutable.js, and React.PureComponent everywhere, and have everything pretty locked down so only the precise components I want to re-render will.

I traced the issue back to ReduxAuthWrapper and discovered that commenting out this line: https://github.com/mjrussell/redux-auth-wrapper/blob/master/src/helper/redirect.js#L50 (and fixing the subsequent issues) appears to do away with my re-renders. However, it also kills the primary function of ReduxAuthWrapper.

Before I go further with this, I wanted to see if there are any ideas about what I might be doing wrong, or any way to get around these constant re-renders. Because I use redux-form my global state is constantly changing, but I scope all my selectors to the specific domains. For some reason I can’t accomplish that here, perhaps because of the way ReduxAuthWrapper itself connects to the global state?

Just for reference, here’s what my implementation looks like — it’s fairly verbatim from the docs and examples here:

export const userIsAuthenticated = connectedRouterRedirect({
  redirectPath: '/login',
  authenticatedSelector: state => userSelector(state).isLoggedIn,
  authenticatingSelector: state => userSelector(state).isLoggingIn,
  AuthenticatingComponent,
  wrapperDisplayName: 'UserIsAuthenticated'
});

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 6
Comments: 24 (10 by maintainers)

Commits related to this issue

Resolves #187 Adds prop mapper to prevent against unnecessary re-rendering for redirection — committed to mjrussell/redux-auth-wrapper by mjrussell 7 years ago
Resolves #187 Adds prop mapper to prevent against unnecessary re-rendering for redirection — committed to mjrussell/redux-auth-wrapper by mjrussell 7 years ago
A better approach to fixing #187 Instead of threading through the redirection from the top, connect it to the redirect which will be only rendered if redireciton occurs — committed to mjrussell/redux-auth-wrapper by mjrussell 7 years ago
Merge pull request #203 from mjrussell/rerender-improv-fix A better approach to fixing #187 — committed to mjrussell/redux-auth-wrapper by mjrussell 7 years ago

Most upvoted comments

Hey everyone,

Im very sorry for the long delay, I think a bit of an explanation is in order. I did a tremendous amount of work for my limited amount of free time getting version 2 pushed and through the door and was feeling accomplished but a little burnt out on completion. Then this re-render bug popped up and although #191 solved some cases, it really didn’t solve the problems listed above. So I stepped away from redux-auth-wrapper and open source work to recharge a little bit.

Coming back, I had a small inspiration and think I’ve fixed the issue in #203. The test I wrote that I couldn’t get fully working in #191 looks good in #203 so I think it should address any issues caused by redux-auth-wrapper. Thats not to say that react-router might throw in some props that cause re-rendering but adding in redux-auth-wrapper shouldn’t trigger any additional re-renders. Please let me know if that is not the case, otherwise I’ll merge the PR in soon and get a new release out there.

mjrussell on Sep 26, 2017

Thanks everyone for all the support, really appreciate it. Version 2.0.2 was just pushed with this fix so it should be all set!

mjrussell on Sep 29, 2017

I’m seeing the same issue as @mdsecurity — the components that I’m decorating aren’t just re-rendering, but re-mounting as well. In that case, I don’t think shouldComponentUpdate will work?

jakelazaroff on Aug 31, 2017

Just want to say thank you for putting in work on this — it’s something you do in your free time, you shouldn’t have to apologize for getting burnt out or wanting a break 🙂 we all appreciate it!

jakelazaroff on Sep 26, 2017

@mjrussell Just want to say thank you, we appreciate all your work, it’s really help us a lot. Take care of yourself first, it’s really the most important thing in your life. Sincerely. 😆

yeluolei on Sep 27, 2017

Sorry all, was on vacation. I’ll get a new version out by the end of the week

mjrussell on Aug 9, 2017

I am also getting an issue where my container is continuously remounting my component has this `componentWillMount(){ changeMyReduxStateVariable(); }

…

mapStateToProps(state){ return { variable:state.MyReduxStateVariable } } return <Route key={index} exact={route.exact} path={route.path} component={userIsAdmin(route.component)}`

when i remove the userIsAdmin it works fine

majodu on Jul 31, 2017

Let me think about this one for a little, I definitely want to push a fix soon but Im concerned that SCU at the top would prevent sub trees from reacting to prop changes properly. I think a possibly more stable alternative would be to include propMappers again, and by default omit props such as redirect from being passed to children.

mjrussell on Jul 16, 2017