Fix shouldComponentUpdate in OverflowList

[braeden]

Checklist

• [ ] Includes tests
• [ ] Update documentation

Changes proposed in this pull request:

Potentially fixes two issues: incorrect logic & lack of prop equality checking.

Potentially incorrect logic:

Original expression: !(this.state !== nextState && shallowCompareKeys(this.state, nextState)) With DeMorgan's laws, this can be written as: (this.state === nextState || !shallowCompareKeys(this.state, nextState))

This is saying that the component SHOULD update iff: the states are equal by reference OR the keys are NOT shallow equal.

I don't believe this was the intended logic, based on the above comments -- I imagine we don't actually want to re-render if the states have referential equality.

I think the intended logic was: this.state !== nextState && !shallowCompareKeys(this.state, nextState) or equivalently !(this.state === nextState || shallowCompareKeys(this.state, nextState))

Lack of prop equality check

If the render's callbacks change reference equality we aren't guaranteed a re-render like we should. This lead to strange bugs, and where breakpoints were accidentally fixing the race conditions happening here.

Reviewers should focus on:

• Whether the new logic I wrote is the intended behavior.
• If there was some deeper reason for omitting the reference equality check on props?

Screenshot

[palantirtech]

Thanks for your interest in palantir/blueprint, @braeden! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.