Skip to content
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

Sometimes when scrolling through table rows, I receive warning: Failed prop type: The prop index is marked as required in FixedDataTableRowImpl, but its value is null. #644

Closed
pauliusg opened this issue Jun 22, 2022 · 4 comments
Labels
bug Onboarding PRs or issues which serve as a good onboarding task

Comments

@pauliusg
Copy link

pauliusg commented Jun 22, 2022

In my case I receive warning when there are more rows to fit in the screen and I scroll to the bottom then start to scroll back to the top. In most times this is enough to get warning but not always.

index.js:1 Warning: Failed prop type: The prop `index` is marked as required in `FixedDataTableRowImpl`, but its value is `null`.
    at FixedDataTableRowImpl (http://localhost:3011/merch/static/js/vendors~main.chunk.js:46437:5)
    at FixedDataTableRow (http://localhost:3011/merch/static/js/vendors~main.chunk.js:46917:5)
    at div
    at FixedDataTableBufferedRows (http://localhost:3011/merch/static/js/vendors~main.chunk.js:44209:5)
    at div
    at div
    at FixedDataTable (http://localhost:3011/merch/static/js/vendors~main.chunk.js:42937:5)
    at ScrollContainer (http://localhost:3011/merch/static/js/vendors~main.chunk.js:47930:5)
    at FixedDataTableContainer (http://localhost:3011/merch/static/js/vendors~main.chunk.js:46105:5)
    at Table (http://localhost:3011/merch/static/js/main.chunk.js:9559:3)
    at div
    at DynamicWidth (http://localhost:3011/merch/static/js/main.chunk.js:9774:3)
    at TableContainer (http://localhost:3011/merch/static/js/main.chunk.js:10142:3)
    at Ue (http://localhost:3011/merch/static/js/main.chunk.js:11531:9)
    at Ge (http://localhost:3011/merch/static/js/main.chunk.js:11605:13)
    at Route (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168474:29)
    at Switch (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168676:29)
    at div
    at push.../../ws-core/ws-core-ui/dist/index.js.exports.TaskRoutes (http://localhost:3011/merch/static/js/main.chunk.js:11904:13)
    at TaskRoutes (http://localhost:3011/merch/static/js/main.chunk.js:101258:3)
    at Route (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168474:29)
    at Switch (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168676:29)
    at ErrorBoundary (http://localhost:3011/merch/static/js/main.chunk.js:95757:5)
    at Oe (http://localhost:3011/merch/static/js/main.chunk.js:11312:13)
    at Re (http://localhost:3011/merch/static/js/main.chunk.js:11369:13)
    at push.../../ws-core/ws-core-ui/dist/index.js.exports.CoreContainer (http://localhost:3011/merch/static/js/main.chunk.js:11878:13)
    at AdminLayout (http://localhost:3011/merch/static/js/main.chunk.js:94648:1)
    at AdminRouter (http://localhost:3011/merch/static/js/main.chunk.js:95092:1)
    at Route (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168474:29)
    at ProtectedRouteBase (http://localhost:3011/merch/static/js/main.chunk.js:7125:1)
    at ProtectedRoute (http://localhost:3011/merch/static/js/main.chunk.js:66514:5)
    at WithAppContextFunctions (http://localhost:3011/merch/static/js/main.chunk.js:69329:7)
    at Switch (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168676:29)
    at http://localhost:3011/merch/static/js/vendors~main.chunk.js:117447:23
    at AdminRoot
    at Router (http://localhost:3011/merch/static/js/vendors~main.chunk.js:168105:30)
    at Provider (http://localhost:3011/merch/static/js/vendors~main.chunk.js:165342:20)
    at AppComponent
    at ContextProvider (http://localhost:3011/merch/static/js/vendors~main.chunk.js:54205:25)

If, I look at FixedDataTableBufferedRows.js code, I see that index can be null:

// if the row doesn't exist in the buffer set, then take the previous one
if (rowIndex === undefined) {
rowIndex =
this._staticRowArray[i] && this._staticRowArray[i].props.index;
if (rowIndex === undefined) {
this._staticRowArray[i] = null;
continue;
}
}

Debugging also approves this situation:
image
image

The biggest issue is not a warning in FixedDataTableRow.js but that table cell with rowIndex=null is rendered:

content = props.cell(cellProps);

image

When documentation in https://github.com/schrodinger/fixed-data-table-2 says:
The cell components in a column will receive the current array index of your data as a prop (this.props.rowIndex). Use this to access the correct value for each cell.

const rows = [0,1,2];
    
<Column
  header={<Cell>Column 1</Cell>}
  cell={({rowIndex, ...props}) => (
    <Cell {...props}>
    {rows[rowIndex]}
    </Cell>
  )}
  width={2000}
/>

So, in my case I get rows[rowIndex]=undefined...

I can fix this for my case, but I guess that in such case cell renderer should not be called.

@pradeepnschrodinger
Copy link
Collaborator

Hey, thanks for the detailed analysis.
As you mentioned, I suspect we just need a broader falsey check on rowIndex and force the row instance to not be rendered in this case.

I'll be happy to consider a PR for this. If not, I can also tackle this myself in a few days.

Meanwhile, could you also mention the exact version of FDT that's used here?

@pradeepnschrodinger pradeepnschrodinger added bug Onboarding PRs or issues which serve as a good onboarding task labels Jun 22, 2022
@pauliusg
Copy link
Author

Hi @pradeepnschrodinger, thank you for quick response. Unfortunately, I don't have more time to make PR. I am using latest FDT version 1.2.9.

@tkirda
Copy link
Contributor

tkirda commented Jun 22, 2022

Hi @pradeepnschrodinger, you were correct. There is a flaw in logic when assigning rowIndex. In case when _staticRowArray[i] already assigned to null, we should keep it as undefined so it can skip rendering that row.

pradeepnschrodinger pushed a commit that referenced this issue Jun 23, 2022
Fixed an edge case where rowIndex is sometimes set to `null`.
@pradeepnschrodinger
Copy link
Collaborator

This should now be resolved with v1.2.10 through #645.

Thanks @pauliusg and @tkirda!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Onboarding PRs or issues which serve as a good onboarding task
Projects
None yet
Development

No branches or pull requests

3 participants