shadcn-vue icon indicating copy to clipboard operation
shadcn-vue copied to clipboard

[Bug]: typecheck bug on the DataTableFacetedFilter example

Open maelp opened this issue 1 year ago • 14 comments

Reproduction

None

Describe the bug

When using the code from the Task table example DataTableFacetedFilter I have the following issue

image
<Command
        :filter-function="
          (list: DataTableFacetedFilter['options'], term) =>
            list.filter((i) => i.title.toLowerCase()?.includes(term))
        "
      >

I'm using the latest radix-vue, but it seems that filter-function first parameter should use "AcceptableValue[]" list, which contains string, number, object etc, but it doesn't typecheck for an arbitrary object for some reason(?)

System Info

System:
    OS: macOS 14.1.2
    CPU: (8) arm64 Apple M1
    Memory: 95.53 MB / 8.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    Yarn: 1.16.0 - ~/.node/bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: 8.15.0 - ~/Library/pnpm/pnpm
    Watchman: 2024.01.22.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 121.0.6167.139
    Safari: 17.1.2
  npmPackages:
    @vueuse/core: 10.7.1 => 10.7.1
    radix-vue: 1.4.0 => 1.4.0
    vue: 3.4.7 => 3.4.7

Contributes

  • [ ] I am willing to submit a PR to fix this issue
  • [ ] I am willing to submit a PR with failing tests

maelp avatar Feb 05 '24 20:02 maelp

@maelp can you hover over underlined red text and show error too, please)

hrynevychroman avatar Feb 08 '24 21:02 hrynevychroman

image

maelp avatar Feb 08 '24 21:02 maelp

image

maelp avatar Feb 08 '24 22:02 maelp

thanks, will take a look on it)

hrynevychroman avatar Feb 08 '24 22:02 hrynevychroman

BTW adding ":string" to term doesn't fix the bug for me (taking a look at your PR)

maelp avatar Feb 08 '24 22:02 maelp

image

maelp avatar Feb 08 '24 22:02 maelp

Yes, it just little issue so never mind, I will remove draft status when resolve it) https://github.com/radix-vue/shadcn-vue/issues/327#issuecomment-1935001998

hrynevychroman avatar Feb 08 '24 22:02 hrynevychroman

@maelp To fix this issue you need to define Option interface globally and pass it inside ComboboxRootProps in Command.vue, this will remove error inside your editor

image image image

Need to think about how to fix it globally, because issue is inside radix-vue components, when you don't define T it takes default value of AcceptableValue which is

export type AcceptableValue = string | number | boolean | object;

It's logical that Option extends object, but VS Code internal TS Server don't think so 😞

Will try to think with it, maybe need to fix radix-vue TS types 🤔

@sadeghbarati maybe you will look and have some ideas, I will appreciate it ❤️

hrynevychroman avatar Feb 08 '24 22:02 hrynevychroman

Interesting... indeed I didn't know whether the issue was that the method is expecting any AcceptableValue and typescript is complaining that we are only handling the case where the filterFunction parameter is of type DataTableFacetedFilter['option'] and not the other cases that it thinks could happen (eg string, number, etc)... wondering if that's the issue? in that case it should be generic?

maelp avatar Feb 08 '24 22:02 maelp

No, issue is that typescript don't think that our custom object for option is object 😁

In my codebase I never use object definition, because it not just Objects. Functions, Arrays is also type of object is JS, think that maybe replace object to type Record<string, any> will help with this 🤔

hrynevychroman avatar Feb 08 '24 22:02 hrynevychroman

Is this fixed? I am still getting the type error on filterFunction. Not sure how to get it right..

image

tuanalumi avatar May 14 '24 16:05 tuanalumi

I'm also getting the same issue...

export type FilterItem = {
  value: string;
  label: string;
};

const onSift = (list: FilterItem[], term: string) => {
  return list.filter(item => item.label.toLowerCase().includes(term.toLowerCase()));
};

Diagnostics:
1. Type '(list: FilterItem[], term: string) => globalThis.FilterItem[]' is not assignable to type '(val: string[] | number[] | false[] | true[] | Record<string, any>[], term: string) => string[] | number[] | false[] | true[] | Record<string, any>[]'. [2322]

9mm avatar Jun 25 '24 18:06 9mm

@maelp To fix this issue you need to define Option interface globally and pass it inside ComboboxRootProps in Command.vue, this will remove error inside your editor

image image image Need to think about how to fix it globally, because issue is inside radix-vue components, when you don't define T it takes default value of AcceptableValue which is

export type AcceptableValue = string | number | boolean | object;

It's logical that Option extends object, but VS Code internal TS Server don't think so 😞

Will try to think with it, maybe need to fix radix-vue TS types 🤔

@sadeghbarati maybe you will look and have some ideas, I will appreciate it ❤️

@romanhrynevych so is the other part of this solution commenting out modelValue still correct, or was that for debugging? I dont want to break the component with unexpected side effects, but otherwise I get:

Diagnostics:

  1. Type 'string' is not assignable to type '(props: LooseRequired<ComboboxRootProps<FilterItem> & { class?: any; }>) => FilterItem | FilterItem[]'. [2322]

9mm avatar Jun 26 '24 01:06 9mm

@maelp To fix this issue you need to define Option interface globally and pass it inside ComboboxRootProps in Command.vue, this will remove error inside your editor

image image image Need to think about how to fix it globally, because issue is inside radix-vue components, when you don't define T it takes default value of AcceptableValue which is

export type AcceptableValue = string | number | boolean | object;

It's logical that Option extends object, but VS Code internal TS Server don't think so 😞

Will try to think with it, maybe need to fix radix-vue TS types 🤔

@sadeghbarati maybe you will look and have some ideas, I will appreciate it ❤️

This fixed the error for me.

In types/index.d.ts

export interface Option {
  label: string;
  value: string;
  icon?: Component;
}

In Command.vue

import { Option } from "@/types";

const props = withDefaults(
  defineProps<
    ComboboxRootProps<Option> & { class?: HTMLAttributes["class"] }
  >(),
  {
    open: true,
  },
);

In FacetedFilter.vue (or wherever you define your filters)

interface DataTableFacetedFilter {
  column: Column<any, any>;
  title: string;
  options: Option[];
  default?: string | boolean;
  search?: boolean;
}
<Command
        :filter-function="
          (list: DataTableFacetedFilter['options'], term: string) =>
            list.filter((row) => row.label.toLowerCase()?.includes(term))
        "
      >

kratos-digital avatar Sep 16 '24 14:09 kratos-digital