table icon indicating copy to clipboard operation
table copied to clipboard

[v8] getValue should return a null value for nested keys

Open dennemark opened this issue 2 years ago • 4 comments

Describe the bug

The issue is related to this issue: https://github.com/TanStack/table/issues/4320#issuecomment-1298293299 The prior issue was solved by introducing an elaborate error message for undefined nested keys.

In a case, where some rows might have nested keys and others not, it is possible to catch the error of getValue, it is not nice though. Furthermore, this also adds up to other functions like getRowModel, when filters are used. So each function would need to get a try catch.

try { cell.getValue() } catch { console.error("🙈") }

The alternative approach would be, to create an entry for each nested key in each row. But it can be annoying for nested objects.

data.forEach((entry)=>{
  if(!(nestedKey0 in entry)){
     entry[nestedKey0] = {}
   }
   if(!(nestedKey1 in entry)){
      entry[nestedKey0][nestedKey1] = null;
   }
})

I would propose returning a null value, if the nested key is missing.

https://github.com/TanStack/table/blob/7b9295332306abcc18f23f96be20fe020862c7dd/packages/table-core/src/core/column.ts#L58

for (const key of accessorKey.split('.')) {
   if(!(key in result)){
     return null
    }
    result = result[key]
    if (process.env.NODE_ENV !== 'production' && result === undefined) {
       throw new Error(
       `"${key}" in deeply nested key "${accessorKey}" returned undefined.`
        )
     }
}

Your minimal, reproducible example

https://codesandbox.io/s/dark-sky-pwysi7?file=/src/main.tsx

Steps to reproduce

See codesandbox. Toggle line 29 comment in main.tsx and see how it works/fails.

I am not using columnHelper, since my use case has a nested Record<string, any> object without proper typing.

Expected behavior

If getValue() returns null for nested keys, it won´t fail.

How often does this bug happen?

No response

Screenshots or Videos

No response

Platform

  • all

react-table version

v.8.5.22

TypeScript version

No response

Additional context

I am not fully sure, if getValue() should return null or undefined, since I am not familiar enough with tanstack table and possible side effects.

Terms & Code of Conduct

  • [X] I agree to follow this project's Code of Conduct
  • [X] I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.

dennemark avatar Nov 01 '22 13:11 dennemark

I'm just trying to migrate from v7 and ended up coming across this on v8.7.0. It seems to me the table shouldn't throw an error if value is undefined on the data level (not when rendering it, as React doesn't allow to render an undefined value).

This was introduced here: https://github.com/TanStack/table/commit/74be9274c38152187ee03255bd2f41b662356640

rasgo-cc avatar Dec 20 '22 17:12 rasgo-cc

I have some nested keys that are sometimes returned as undefined or null from my backend, both of which mean different things about that data, so it would not be great if it was always converted to a null by getValue().

GRA0007 avatar Feb 20 '23 07:02 GRA0007

I really want to use this table extension in my vue project. But I have optional returned parameters in my response from server. So I know that there are deeply nested keys which are undefined / does not exists. The extensions spams my whole console every time I do an interaction with it... "updated" in deeply nested key "attributes.updated" returned undefined.

Is there any possibility to at least disable the warning messages? Will it also work in production?

I would be very happy about an answer, otherwise I unfortunately can not use the extension. Can I ask kindly for your opinion @tannerlinsley?

marciado avatar May 09 '23 13:05 marciado

I have a work around that might help people

columnHelper.accessor( (row) => row?.mostRecentAnalysis?.risk, { header: t('plots.table.risk') ?? '', cell: (info) => ( <StatusIndicator text={deforestationRiskMapping(info.getValue())} /> ), } ),

paulblackmore avatar Mar 07 '24 13:03 paulblackmore