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

Error when global filter called for numeric columns #4210

Closed
2 tasks done
lexanth opened this issue Jul 22, 2022 · 9 comments
Closed
2 tasks done

Error when global filter called for numeric columns #4210

lexanth opened this issue Jul 22, 2022 · 9 comments

Comments

@lexanth
Copy link
Contributor

lexanth commented Jul 22, 2022

Describe the bug

#4124 (fixed by a5578ac) makes the table now call the global filter functions for strings and numbers.

However, most of the built-in filter functions don't handle numbers, they assume the value is a string (https://github.com/TanStack/table/blob/main/packages/table-core/src/filterFns.ts)

This makes it very liable to break (e.g. auto filter does includesString, which breaks on toLowerCase). The workaround is easy - provide a custom filter function for numeric columns or exclude them from the filtering.

It would be nicer if the default filter functions handled numbers

Your minimal, reproducible example

https://nyxdj8.sse.codesandbox.io

Steps to reproduce

  1. Type "a" in the global filter
  2. See Uncaught TypeError: _row$getValue.toLowerCase is not a function in the console (filtering doesn't complete)

Expected behavior

The default filter functions should handle numbers by default

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

macOS, Chrome

react-table version

8.3.0

TypeScript version

4.6.3

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • 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.
@marceloverdijk
Copy link
Contributor

While upgrading from 8.2.x to 8.3.3 I'm experiencing the same.

      {
        id: 'matches',
        header: 'Matches',
        accessorFn: (originalRow) => originalRow.matches.toString(), // matches is a number
        // accessorKey: 'matches', // this worked before
        sortDescFirst: true, // and I also had to add this explicitly
      },

@fabiendeborde
Copy link

Just stumbled upon this and found a way around the error.
Using the latest ColumDefs documentation, I updated my numeric columns to display a string instead of a number (since it doesn't really make sense to use a number here anyway...)
Now the global filter is working without having to provide a custom filter function.

const columns: ColumnDef<User>[] = [
  columnHelper.accessor(row => row.id.toString(), {
    id: 'id',
    header: 'User ID'
  }),
  ...
]

@marceloverdijk
Copy link
Contributor

@fabiendeborde i don't think using the accessor is the smartest way to do this.
In fact this will impact your sorting, as it will now sort by the string value and not the number value..
Imagine number values like: 10, 101, 20

@jeg924
Copy link

jeg924 commented Jul 27, 2022

Does "2 tasks done" mean someone is working on a fix?

@fabiendeborde
Copy link

@marceloverdijk I was worried about that too, but the default sorting is working just fine...
To me, since the search input is also a string, it doesn't shock me much to cast the number data to a string, as long as the sorting keeps working.
(Entering a string and expecting the filter function to find a number feels a bit too "magical" to me)

@jeg924
Copy link

jeg924 commented Jul 28, 2022

@fabiendeborde Thanks for your work, you solved my problem!

@marceloverdijk
Copy link
Contributor

@tannerlinsley was there a fix for this? as with latest 8.5.5 I'm still getting a l.toLowerCase is not a function error when using a global filter with numeric columns...

@marceloverdijk
Copy link
Contributor

@marceloverdijk I was worried about that too, but the default sorting is working just fine... To me, since the search input is also a string, it doesn't shock me much to cast the number data to a string, as long as the sorting keeps working. (Entering a string and expecting the filter function to find a number feels a bit too "magical" to me)

Yes I noticed this as well for some of my tables, that the sorting was working just fine as expected (sorting by number and not string). However it's a bit strange it's working like that to be honest --> as we explicitly converted to string...

Now the funny part is I see this behaviour for all my tables except for just 1...

image

Breaking my head on this one ;-)

@liubomyr123
Copy link

Just stumbled upon this and found a way around the error. Using the latest ColumDefs documentation, I updated my numeric columns to display a string instead of a number (since it doesn't really make sense to use a number here anyway...) Now the global filter is working without having to provide a custom filter function.

const columns: ColumnDef<User>[] = [
  columnHelper.accessor(row => row.id.toString(), {
    id: 'id',
    header: 'User ID'
  }),
  ...
]

Thanks, helped me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants