T
TanStack•3y ago
helpful-purple

table controlled / uncontrolled state the treatment of undefined

it seems that as soon as I pass an on change handler (I tested expanded and column sizing) the table assumes that this a controlled state the problem is, that the table recognizes undefined as a value. so it's much harder to create a wrapper to the table that can be also controlled or uncontrolled depending on the props. it seems that both an undefined state value and an undefined change handler are recognized as a value and change the behavior of the table. I think it would be nice if undefined would have been treated as a no-value, for example options = { initialState: { expanded: true }, state: { expanded: undefined } } would still expand all rows
5 Replies
helpful-purple
helpful-purpleOP•3y ago
anything?
harsh-harlequin
harsh-harlequin•3y ago
@alissa Can you provide a code sample on codesandbox or something similar ? Need to understand the behaviour.
helpful-purple
helpful-purpleOP•3y ago
@Gopal Panigrahi here is a simple example with expanding rows https://codesandbox.io/p/sandbox/vigorous-christian-ik5m8r?file=%2Fsrc%2Fmain.tsx&selection=%5B%7B%22endColumn%22%3A33%2C%22endLineNumber%22%3A131%2C%22startColumn%22%3A33%2C%22startLineNumber%22%3A131%7D%5D here the onExpandedChange is undefined and you can't collapse rows. if you comment it out you can collapse rows. if you uncomment the 
 state: {
       expanded: undefined,
   },
 state: {
       expanded: undefined,
   },
then the ts passes but it fails in runtime and here is an example with column visibility https://codesandbox.io/p/sandbox/friendly-wing-nwwp2w?welcome=true&file=%2FREADME.md if you comment out 
state: {
      columnVisibility: undefined,
    },
    onColumnVisibilityChange: undefined,
state: {
      columnVisibility: undefined,
    },
    onColumnVisibilityChange: undefined,
you can hide columns I think that undefined should be treated as "no value passed"
vigorous-christian-ik5m8r
CodeSandbox is an online editor tailored for web applications.
friendly-wing-nwwp2w
CodeSandbox is an online editor tailored for web applications.
harsh-harlequin
harsh-harlequin•3y ago
Hi @alissa, Thanks for the examples, I was looking into the examples and investigating the reason for this behaviour. I agree with you that undefined should be treated as no value passed, otherwise the application doesn't work as expected. I was looking into the library to find the issue and it turns out 
state: {
  expanded: undefined,
},
state: {
  expanded: undefined,
},
passes the ts, but fails at runtime because all the types can accomodate undefined. Thus, undefined satisfies, the boolean type assertion placed on expanded variable. Internally, following code snippet fails. 
if (typeof expanded === 'boolean') {
  return expanded === true
}

if (!Object.keys(expanded).length) {
  return false
} 
if (typeof expanded === 'boolean') {
  return expanded === true
}

if (!Object.keys(expanded).length) {
  return false
}
It fails here, as in here we are trying to find keys in undefined. Most appropriate, solution here would be just to place a condition to check for undefined ( and null, as it behaves same for null). Here, it seems to be a logical issue. In case of onExpandedChange, when we pass undefined, it is internally creating defaultOptions object where onExpandedChange is not undefined (there is a function makeStateUpdater()), but while merging the user defined options and defaultOptions, I believe, it is using the undefined, currently, i am not able to understand the entire logic of how it works. But I agree with you, that undefined should have been treated as no value passed, this could be done by either sanitizing any undefined/null value from passed option, or updating the mergeOptions to handle undefined. Maybe, you could share your opinion on it.
helpful-purple
helpful-purpleOP•3y ago
I didn't dig into the table code yet 🙂 normally the easier way is to strip undefined from the passed-in options (for existing code) but I don't know... maybe it was done intentionally. I can look into the code and find a good way to implement it, but first, it will be nice to know if people agree that it should done 🙂

Did you find this page helpful?