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

ConnectedRouter is re-rendering when Redux store updates #205

Closed
jakewies opened this Issue Dec 12, 2018 · 7 comments

Comments

Projects
None yet
6 participants
@jakewies
Copy link

jakewies commented Dec 12, 2018

I recently migrated from the deprecated react-router-redux package to connected-react-router in order to successfully update react-redux to v6. The migration went well enough thanks to the detailed docs for this project.

However, now that I am on v6.0.0 of both connected-react-router and react-redux I am noticing some strange things occurring with the ConnectedRouter component.

First issue

As others have already stated, the ConnectedRouter seems to be rendering two times on initial page load.

Second issue

This is the larger one for me, and I'm still trying to figure out why it is happening. Basically, on any update to the state of my redux store, the ConnectedRouter component re-renders, causing unnecessary re-renders of any child components below it.

I set up an example repo that you can clone to get more details. I'll give a brief description here:

Description

I've setup my redux store according to the docs in this repository. Assuming this, here is a basic example using ConnectedRouter:

// App.js

const App = () => (
  <Provider store={store}>
    <ConnectedRouter history={history}>
      <div>
        <Nav />
        <Switch>
          <Route exact path="/" component={Home} />
          <Route path="/about" component={About} />
        </Switch>
      </div>
    </ConnectedRouter>
  </Provider>
)

The Home component renders a child IncrementCount component which dispatches an action to increment the count value stored in redux store:

// IncrementCount.js

import { increment } from './actions'

const IncrementCount = ({ increment }) => (
  <button onClick={increment}>Increment</button>
)

export default connect(null, { increment })(IncrementCount)

The About component renders a child CountValue component which displays the current count:

// CountValue.js

const CountValue = ({ value }) => (
  <div>{value}</div>
)

const mapStateToProps = ({ app }) => ({
  value: app.count
})

export default connect(mapStateToProps)(CountValue)

When incrementing the count on the '/' route, the ConnectedRouter re-renders along with the Nav component (doesn't even connect to the redux store), and the Home component.

This is not the expected behavior.

The expected behavior occurs when replacing ConnectedRouter with BrowserRouter from the react-router-dom library:

// with BrowserRouter

const App = () => (
  <Provider store={store}>
    <BrowserRouter>
      <div>
        <Nav />
        <Switch>
          <Route exact path="/" component={Home} />
          <Route path="/about" component={About} />
        </Switch>
      </div>
    </ConnectedRouter>
  </Provider>
)

In the code above, incrementing the count value on the '/' route does not cause any unnecessary re-renders to the Nav or Home components.

@merildev

This comment has been minimized.

Copy link

merildev commented Dec 12, 2018

Same bug here, I tried different ways of rendering the component (prop, children prop, render prop) and it does not have any effect. The issue seems to be linked to connected-react-router.

@radziksh

This comment has been minimized.

Copy link

radziksh commented Dec 12, 2018

@jakewies @merildev this workaround works for me:

I found that changing any routes from using component prop to render prop did the trick:

Works

        <Switch>
          <Route path={'login'} render={props => <LoginContainer {...props} />} />
        </Switch>
Renders 2x

        <Switch>
          <Route path={'login'} component={props => <LoginContainer {...props} />} />
        </Switch>

#193 (comment)

@jakewies

This comment has been minimized.

Copy link

jakewies commented Dec 12, 2018

@radziksh This has no affect on either of the problems this issue is trying to examine.

What versions of react-redux and connected-react-router are you using? The comment you posted is coming from an issue that references a different version of connected-react-router.

@Annatsu

This comment has been minimized.

Copy link
Contributor

Annatsu commented Dec 14, 2018

@jakewies i cloned your repository to make some tests and try to fix the problem.

Looks like the problem was indeed inside the ConnectedRouter component.

If you look at the file, in the function createConnectedRouter, it creates the ConnectedRouter extending React.Component.

Then, it creates a Container Component called ConnectedRouterWithContext, that connects it's children with the redux Context.

What happens is that ConnectedRouterWithContext is a functional component, and ConnectedRouter is just a React.Component class. They don't have shouldComponentUpdate implementations.

This is probably what's causing the re-renders. I've forked this repo, switched the ConnectedRouter to a PureComponent, and it seemed to work just like expected (like the BrowserRouter example).

@Annatsu

This comment has been minimized.

Copy link
Contributor

Annatsu commented Dec 14, 2018

Hey guys! I've opened #208 to hopefully fix this problem 😄

@foffer

This comment has been minimized.

Copy link

foffer commented Dec 16, 2018

FWIW I'm getting an

Invariant Violation: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops. error when calling setState in componentDidMount due to the multiple renders

@supasate

This comment has been minimized.

Copy link
Owner

supasate commented Dec 27, 2018

It should be fixed in v6.1.0. Please let me know if it helps and feel free to re-open this issue if the problem still exists.

Thank you all for your contributions!

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