search

KevinVandy / material-react-table

A fully featured Material UI V5 implementation of TanStack React Table V8, written from the ground up in TypeScript
https://material-react-table.com
MIT License
1.52k stars 439 forks source link

accessorFn is called in loading state with dummy skeleton rows #721

Closed BendixP closed 1 year ago

BendixP commented 1 year ago

material-react-table version

1.15.0

react & react-dom versions

18.2.0

Describe the bug and the steps to reproduce it

When the table is in a loading state and showing skeleton rows, the accessor funtion is called with the dummy row, that has undefined values for all its relevant properties. This was not the case in previous versions. It causes problems because the type of accessorFn doesn't necessarily take into account the possibility of undefined values coming in. Probably the accessorFn should not be called at all in the loading state.

Minimal, Reproducible Example - (Optional, but Recommended)

https://codesandbox.io/p/sandbox/recursing-tristan-55ph8q?file=%2Fpackage.json%3A11%2C5

Screenshots or Videos (Optional)

No response

Do you intend to try to help solve this bug with your own PR?

Maybe, I'll investigate and start debugging

Terms

KevinVandy commented 1 year ago

fixed in v2, backported to v1.15.1

BendixP commented 1 year ago

Unfortunately there is still a problem when sorting is not disabled: https://codesandbox.io/p/sandbox/dawn-sound-64tkh7?file=%2Fsrc%2FTS.tsx%3A41%2C31 Should i open a new issue?

dkuulis commented 1 year ago

With deep accessor keys also get undefined value warnings from https://github.com/TanStack/table/blob/a1e9732e6fc3446a2ae80db72a1f2b46a5c11e46/packages/table-core/src/core/column.ts#L101

KevinVandy commented 1 year ago

With deep accessor keys also get undefined value warnings from https://github.com/TanStack/table/blob/a1e9732e6fc3446a2ae80db72a1f2b46a5c11e46/packages/table-core/src/core/column.ts#L101

That's actually by design. In fact, we used to throw an error in this situation in earlier versions. If you have inconsistent data structures that need to be handled, I recommend using an accessorFn instead of accessorKey so that you can handle optional chaining and fallback values gracefully how you need to.

KevinVandy commented 1 year ago

Unfortunately there is still a problem when sorting is not disabled: https://codesandbox.io/p/sandbox/dawn-sound-64tkh7?file=%2Fsrc%2FTS.tsx%3A41%2C31 Should i open a new issue?

Just found what I needed to short-circuit in the sorting tooltip for that. Will be fixing it immediately for v2. Adding optional chaining in your accessorFn also fixes this as when blank cells are created, they are given all null values in every column, which causes problems when you call string methods. TypeScript is not warning you about this, so easy to make this error.

BendixP commented 1 year ago

Just found what I needed to short-circuit in the sorting tooltip for that. Will be fixing it immediately for v2. Adding optional chaining in your accessorFn also fixes this as when blank cells are created, they are given all null values in every column, which causes problems when you call string methods. TypeScript is not warning you about this, so easy to make this error.

Will that fix be backported as well?

KevinVandy commented 1 year ago

I'll probably do one more batch of backported fixes. I planning to release v2 as stable this weekend though

drewterry commented 11 months ago

This is noted as fixed in the v1.15.1 changelog, but does not appear to have been backported.

KevinVandy commented 11 months ago

This is noted as fixed in the v1.15.1 changelog, but does not appear to have been backported.

Ok yes. If sorting is enabled, the bug is still there. But just update to the latest v2 release and it's fixed.