Fix scrolling - Beta

[pradeepnschrodinger]

Description

This is going to be hard to explain. Basically this fixes the issue that users mentioned in #460.
The bug only seems to affect a certain set of people, and until now we had no way of reproducing it.

Few hours ago a user created the issue (#482) , and gave a minimal sandbox.

After a lot of working around, I found out that the react version used there is 16.8.6. If we roll it back to 16.0.0-alpha.2 or less, the bug doesn't occur.

Next step was finding why this was a bug only after #460. Turns out there was whitespace in the render method of FixedDataTableBufferedRows. As harmless as it seems, that single space (actually two of them, left and right), introduced a different render tree.

Before the whitespace, the render tree was like:

Buffered Rows -> {
  <div>
    [ array of rows ]
  </div>
}

After the whitespace, the render tree was like:

Buffered Rows -> {
  <div>
    [
       ' ',
       [ array of rows ],
       ' ',
    ]
  </div>
}

Now this is my understanding: after React 16, components in the tree won't be considered for rendering if the same object is passed in (checked through shallow equal comparison).

So in the first case, we take the array inside the parent div, shallow equal compare the elements, and see that they are all changed through each scroll. Hence React renders them.

In the second case, however, if we take the array inside the parent div, shallow equal compares the elements, and finds that between renders, they're all the same.

Why? The object [ array of rows ] is mutable, (see this._staticRowArray usage), and hence the object reference won't change between renders. This makes react think that the tree is static, and skips rendering. This is only seen in v16 and up (more specifically from 16.0.0-alpha.2 and above from my quick testing).

Motivation and Context

Fixes #482

How Has This Been Tested?

Example site with latest React 16.8.6

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.

[pradeepnschrodinger]

Also special thanks to @ashwinb7 for helping me wrap around how React skips the update because of the shallow equal check.

[pradeepnschrodinger]

[pradeepnschrodinger]

[wcjordan]

Interesting, wow thanks for tracking this down.

Do you know if we would be able to catch this w/ a unit test if we had one which ran with a React 16 version? It may be worth ticketing out a follow up to add that.

[pradeepnschrodinger]

Do you know if we would be able to catch this w/ a unit test if we had one which ran with a React 16 version? It may be worth ticketing out a follow up to add that.

Yea, I think we'll probably need snapshots or a way to make sure that the component got rendered into the DOM with the correct translate style or something 🤔
Anyway, will ticket it out.

[pradeepnschrodinger]

Released with v1.0.0-beta.17.