shouldComponentUpdate() not working on development when used with Router

[coluccini]

Hi!
I'm trying to implement shouldComponentUpdate() method on some components to avoid unnecessaries renders and it is totally ignored while in development. The render method is triggered even if I return a straight false in it. But it works as expected in the production build...
So I've dug around and it looks like when using Router to change the url (even if you stay in the same page and use shallow routing) everything gets re renders without taking shouldComponentUpdate into account. But only in development.

I've reproduced this in the with-redux example making this 2 changes to components/AddCount.js file:

class AddCount extends Component {
  add = () => {
    this.props.addCount()

    // Change 1
    Router.push('/', `/${this.props.count}`, {shallow: true})
  }

  // Change 2
  shouldComponentUpdate() {
    return false;
  }

  render () {
    const { count } = this.props
    return (
      ...
    )
  }
}

If you run this in devel the count would increase and it will stay the same on a production build.
It this the expected behaviour? is there's a way to avoid it?

PS: same think happens using PureComponent or with Recompose HOC, like pure(), shouldUpdate(), etc.

[TooTallNate]

I'm bumping into this as well 👍

[arunoda]

Can someone send me a sample repo please ? 🙏

[coluccini]

Of course: https://github.com/coluccini/next-with-redux
The AddCount component is returning false within shouldComponentUpdate() but it is still rendering in Dev and working ok on a Prod build.
Digging into the Next.js code I found the following comment:

/lib/app.js

// includes AppContainer which bypasses shouldComponentUpdate method
// https://github.com/gaearon/react-hot-loader/issues/442

So, I think, this is where everything starts.

[coluccini]

Another thing I saw is that, on Dev, the return !shallowEquals(this.props, nextProps) in shouldComponentUpdate() in /lib/app.js inside class Container is always returning true because the prop url (passed from App class and created using createUrl(router)) is never strictly equal.

[harshmaur]

Hello,

Even I observe this. shouldComponentUpdate does not seem to work at all in development. It works in production though.

[marbemac]

This just bit us in production (and continues to be a problem) because we're not able to test shouldComponentUpdate logic effectively in development. Something that re-renders appropriately in development might very well not re-render in production.

@arunoda - to add to the above example, I have created an example that reproduces the issue in an isolated environment. It includes instructions for running, and seeing what's going on. It also includes a link to an animated gif for you lazy folks out there :).

https://github.com/stoplightio/nextjs-router-bug

[j]

Any info on this?

The thing we're working on needs to ensure a component never gets modified (ads), and it's hard to manage this without the use of shouldComponentUpdate.

[nelsonomuto]

I am also facing this same issue, I need to make sure shouldComponentUpdate is ran but this is not happening after adding withRouter

[timneutkens]

This should be fixed by #4500, which is currently available on next@canary.