TypeScript updates #1521

ggascoigne · 2019-09-15T19:54:06Z

I've found that trying to get solid TypeScript typings for this library
are a bit of a challenge. The composable nature of the library means
that the types for the builtin functions are by their nature somewhat
minimal, and that the user needs to be able to extend those interfaces
to reflect the specific plugins that are in use.

With that in mind I propose something like this.

To get the best out of them, you then need to extend those interfaces
using declaration merging, see
https://www.typescriptlang.org/docs/handbook/declaration-merging.html

e.g.

declare module 'react-table' {
  export interface TableInstance<D = any>
    extends UseFiltersValues<D>,
      UsePaginationValues<D>,
      UseExpandedValues,
      UseGroupByValues {}

  export interface TableState<D = any>
    extends UsePaginationState,
      UseGroupByState,
      UseSortbyState<D>,
      UseFiltersState<D> {}
}

This also puts the ability to extend those types with any user defined
hooks completely in the users hands.

This gives the greatest flexibility, but I'll admit that it isn't
particularly obvious. Perhaps a typescript readme and example is needed.

I've found that trying to get solid TypeScript typings for this library are a bit of a challenge. The composable nature of the library means that the types for the builtin functions are by their nature somewhat minimal, and that the user needs to be able to extend those interfaces to reflect the specific plugins that are in use. With that in mind I propose something like this. To get the best out of them, you then need to extend those interfaces using declaration merging, see https://www.typescriptlang.org/docs/handbook/declaration-merging.html e.g. ```ts declare module 'react-table-hooks' { export interface TableInstance<D = any> extends UseFiltersValues<D>, UsePaginationValues<D>, UseExpandedValues, UseGroupByValues {} export interface TableState<D = any> extends UsePaginationState, UseGroupByState, UseSortbyState<D>, UseFiltersState<D> {} } ``` This also puts the ability to extend those types with any user defined hooks completely in the users hands. This gives the greatest flexibility, but I'll admit that it isn't particularly obvious. Perhaps a typescript readme and example is needed.

ggascoigne · 2019-09-15T19:55:47Z

@stramel I'd be very interested in your opinion about this.

tannerlinsley · 2019-09-15T21:59:47Z

This definitely looks like a step in the right direction.

stramel · 2019-09-15T22:21:10Z

Will take a look when I get back tomorrow evening

janhartmann · 2019-09-16T06:06:32Z

Thanks for this @ggascoigne, I actually forked the library yesterday to "fix" these typings myself. Happy to see you came just before mine.

I can help out also in the longer run, if needed.

index.d.ts

JimmyMultani · 2019-09-16T14:24:09Z

Wow, this is great @ggascoigne ! I think some plugin hooks are missing, like useRowSelect & useRowState.

ggascoigne · 2019-09-16T17:31:55Z

Wow, this is great @ggascoigne ! I think some plugin hooks are missing, like useRowSelect & useRowState.

agreed. I'll admit that what's here is rather focused on what I've been using, there are clearly some gaps.

I'll try and take a pass at looking at the samples - they might help me flesh out some of the gaps. Perhaps having a couple of the samples be in typescript might be useful anyway.

index.d.ts

codesandbox-ci · 2019-09-18T21:36:38Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b62afa3:

Sandbox	Source
tender-shannon-6tnfg	Configuration

stramel · 2019-09-18T21:49:36Z

index.d.ts

-        sortedDesc: boolean;
-        sortedIndex: number;
-    }
+  export type Filters<D> = {


I think this is the only thing left that I noticed. This can be Record<StringKey<D>, string>

actually this didn't quite work since the key for the filter should be anything that can be an ID (same with sort), and that can include string.

Either way, I pulled out an IdType and then used that.

I'm still not particularly keen on allowing string in that type, it pretty much removes most of the explicit type checking we're jumping through hoops to get, but I also don't see how to avoid it. It's pretty pervasive.

stramel · 2019-09-18T21:50:33Z

index.d.ts

-        getRowProps: () => any;
-        original: any;
-    }
+  type StringKey<D> = Extract<keyof D, string>


stramel · 2019-09-18T21:52:21Z

This is definitely a big improvement and moves us one step closer. Thanks for the PR and working with me on the changes. Other than that one change it LGTM.

ggascoigne · 2019-09-18T22:30:48Z

well that one last change was a bit more than just one line :) But yes, it was a pleasure, thank you.

fxOne · 2019-09-19T06:10:22Z

index.d.ts


-    export function useRows<D>(props: TableOptions<D>): TableOptions<D> & UseRowsValues<D>;
+  export interface UsePaginationValues<D = {}> {


Shouldn't this option also be on the TableInstance as shown in this excample ?

No. This is discussed in the description of this PR. I really need to put together a typescript readme and a couple of examples so that it's clearer how to compose usable types.

janhartmann · 2019-09-19T06:24:39Z

With these typings, how do we get typings for plugins? For instance, usePagination?

By doing this, all pagination related properties are not typed:

const tableState = useTableState<TableState2>({ pageIndex: 2 });
const {
    getTableProps,
    headerGroups,
    prepareRow,
    pageOptions,
    page,
    pageCount,
    pageSize,
    pageIndex,
    gotoPage,
    canNextPage,
    canPreviousPage,
    previousPage,
    nextPage,
    setPageSize
  } = useTable(
    {
      columns,
      data,
      state: tableState,
      loading: false
    },
    usePagination
  );

8enSmith · 2019-09-19T10:20:34Z

index.d.ts

+     */
+    accessor: IdType<D> | AccessorFn<D>
+    Header?: ReactNode | ((props: TableInstance<D>) => ReactNode)
+    Filter?: ReactNode | ((props: TableInstance<D>) => ReactNode)


In the filter example there are two filter properties on the HeadColumn: filter and Filter e.g.

{ Header: 'Status', accessor: 'status', Filter: SelectColumnFilter, filter: 'includes', }

I can see that the TS definition for Filter is present, but the typing for filter appears to be missing. Is it going to be added as part of this PR?

I've raised this observation here, as opposed to creating another issue, as when I previously raised a TS issue I was told that this is the PR for the big TS rework exercise(!).

You're correct, it is missing that. However, I think we could merge these and then tackle covering 100% of the API.

The sooner these changes are merged the better. Using this library with TypeScript without accurate typings can cause confusion.

stramel · 2019-09-19T12:09:46Z

For those concerned, I'm not saying this is THE finalized typings but a step towards the correct and finalized typings. Currently, typings aren't published in this package and the typings included in this package were valid as of alpha.2, so quite outdated.

My vote is we get this in and continue to iterate on it instead of trying to make the typings perfect in one pass.

janhartmann · 2019-09-20T14:24:18Z

I've been playing around with these and here is my current observations. :-)

canSort is missing on the HeaderColumn<T>, but it has the canSortBy which is not present in the API
No support for useRowSelect hook: Missing types
No support for grouped headings, for instance:

    {
        Header: 'Name',
        columns: [
          {
            Header: 'First Name',
            accessor: 'firstName'
          },
          {
            Header: 'Last Name',
            accessor: 'lastName',
 
          },
        ]
      }

There is width, maxWidth etc present on the HeaderColumn which the current API does not support.

I can help by updating these typings to the current API: https://github.com/tannerlinsley/react-table/blob/master/docs/api.md#column-properties-1 but should I branch off this one or wait for the merge and then fork?

stramel · 2019-09-20T15:20:35Z

@tannerlinsley Are you good with merging this as is and iterating on it? It still isn't being published but would at least give us a more accurate representation of the current API.

@janhartmann Hopefully, we can get this merged and you can submit a pr

tannerlinsley · 2019-09-20T15:37:41Z

Yep. I’m good to merge.

tannerlinsley · 2019-09-20T15:37:58Z

Keep working on them!

stramel · 2019-09-20T15:43:51Z

Thanks to @ggascoigne for the hard work on this PR! 👏

8enSmith · 2019-09-20T21:53:17Z

So now that these TypeScript changes have been merged, will an update to the v7 beta branch be released? I am having to heavily work around the deficiencies in the current definition file so an update is eagerly awaited.

stramel · 2019-09-20T22:01:05Z

@8enSmith You should be able to uninstall the @types/react-table package, then provide the types manually. Typically, this is what I do:

Create a custom_typings folder
Modify tsconfig.json to include the newly created custom_typings folder. "include": ["custom_typings/*.d.ts", "src"],
Create a file with the same name as the package, react-table.d.ts.
Insert the typings into that file.

I'm hesitant to publish the types as they are still not complete and there is an easy workaround to using what we currently have.

8enSmith · 2019-09-20T22:13:18Z

Thanks for the prompt advice @stramel . It looks as though it doesn't use @types/react-table, though I guess your suggestion (i.e. custom folder) will still work?

The more I use (and <3 ) TypeScript, it's a wonder that complicated libraries like this don't opt to use TS from the start! I guess using vanilla JS gives more flexibility but at a cost i.e. maintenance pains! O_o

8enSmith · 2019-09-20T22:43:48Z

@8enSmith You should be able to uninstall the @types/react-table package, then provide the types manually. Typically, this is what I do:

Create a custom_typings folder

Modify tsconfig.json to include the newly created custom_typings folder. "include": ["custom_typings/*.d.ts", "src"],

Create a file with the same name as the package, react-table.d.ts.

Insert the typings into that file.

I'm hesitant to publish the types as they are still not complete and there is an easy workaround to using what we currently have.

FYI, I added the following to my tsconfig and its working nicely:

"include": [
    "custom_typings/*.d.ts",
    "src"
  ],
  "paths": {
    "react-table": [
      "custom_typings/react-table.d.ts"
    ]
  }

I added the "paths" entry to override the module definition as suggested here. Thanks again @stramel

ggascoigne · 2019-09-20T23:01:27Z

@janhartmann re. "With these typings, how do we get typings for plugins? For instance, usePagination?" The short answer is do what I suggest in the description of this PR. But I suspect that I need to describe this more clearly.

I'll put together an example and link it.

ggascoigne · 2019-09-21T05:12:41Z

FYI there's a simple example in #1534

michael-land · 2019-09-25T20:48:24Z

index.d.ts

+    columns: HeaderColumn<D>[]
+    state: State<D>
+    debug?: boolean
+    loading: boolean


Is there a loading prop?

There doesn't appear to be a loading prop anymore. See the latest typings that are in progress. #1535

Abdallatif reviewed Sep 16, 2019

View reviewed changes

index.d.ts Outdated Show resolved Hide resolved

fix useExpanded type

4135ff1

ggascoigne added 2 commits September 16, 2019 11:17

fix module name

1c56269

fix typo

5eea6fb

stramel suggested changes Sep 17, 2019

View reviewed changes

stramel mentioned this pull request Sep 18, 2019

Incorrect TypeScript typings specified in EnhancedColumn interface #1530

Closed

fxOne reviewed Sep 18, 2019

View reviewed changes

index.d.ts Show resolved Hide resolved

stramel reviewed Sep 18, 2019

View reviewed changes

index.d.ts Outdated Show resolved Hide resolved

address review feedback

dd058e1

stramel reviewed Sep 18, 2019

View reviewed changes

add IdType, update Filters definition

b62afa3

stramel approved these changes Sep 18, 2019

View reviewed changes

fxOne reviewed Sep 19, 2019

View reviewed changes

8enSmith reviewed Sep 19, 2019

View reviewed changes

tannerlinsley merged commit 7ab6385 into TanStack:master Sep 20, 2019

ggascoigne deleted the ts-updates branch September 21, 2019 03:22

michael-land reviewed Sep 25, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TypeScript updates #1521

TypeScript updates #1521

ggascoigne commented Sep 15, 2019 •

edited

Loading

ggascoigne commented Sep 15, 2019

tannerlinsley commented Sep 15, 2019

stramel commented Sep 15, 2019

janhartmann commented Sep 16, 2019

JimmyMultani commented Sep 16, 2019

ggascoigne commented Sep 16, 2019

codesandbox-ci bot commented Sep 18, 2019 •

edited

Loading

stramel Sep 18, 2019

ggascoigne Sep 18, 2019

stramel Sep 18, 2019

stramel commented Sep 18, 2019

ggascoigne commented Sep 18, 2019

fxOne Sep 19, 2019

ggascoigne Sep 20, 2019

janhartmann commented Sep 19, 2019 •

edited

Loading

8enSmith Sep 19, 2019 •

edited

Loading

stramel Sep 19, 2019

8enSmith Sep 19, 2019

stramel commented Sep 19, 2019

janhartmann commented Sep 20, 2019

stramel commented Sep 20, 2019 •

edited

Loading

tannerlinsley commented Sep 20, 2019

tannerlinsley commented Sep 20, 2019

stramel commented Sep 20, 2019

8enSmith commented Sep 20, 2019

stramel commented Sep 20, 2019 •

edited

Loading

8enSmith commented Sep 20, 2019 •

edited

Loading

8enSmith commented Sep 20, 2019 •

edited

Loading

ggascoigne commented Sep 20, 2019

ggascoigne commented Sep 21, 2019

michael-land Sep 25, 2019

stramel Sep 25, 2019

michael-land Sep 25, 2019


		export function useRows<D>(props: TableOptions<D>): TableOptions<D> & UseRowsValues<D>;
		export interface UsePaginationValues<D = {}> {

TypeScript updates #1521

TypeScript updates #1521

Conversation

ggascoigne commented Sep 15, 2019 • edited Loading

ggascoigne commented Sep 15, 2019

tannerlinsley commented Sep 15, 2019

stramel commented Sep 15, 2019

janhartmann commented Sep 16, 2019

JimmyMultani commented Sep 16, 2019

ggascoigne commented Sep 16, 2019

codesandbox-ci bot commented Sep 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stramel commented Sep 18, 2019

ggascoigne commented Sep 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janhartmann commented Sep 19, 2019 • edited Loading

8enSmith Sep 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stramel commented Sep 19, 2019

janhartmann commented Sep 20, 2019

stramel commented Sep 20, 2019 • edited Loading

tannerlinsley commented Sep 20, 2019

tannerlinsley commented Sep 20, 2019

stramel commented Sep 20, 2019

8enSmith commented Sep 20, 2019

stramel commented Sep 20, 2019 • edited Loading

8enSmith commented Sep 20, 2019 • edited Loading

8enSmith commented Sep 20, 2019 • edited Loading

ggascoigne commented Sep 20, 2019

ggascoigne commented Sep 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggascoigne commented Sep 15, 2019 •

edited

Loading

codesandbox-ci bot commented Sep 18, 2019 •

edited

Loading

janhartmann commented Sep 19, 2019 •

edited

Loading

8enSmith Sep 19, 2019 •

edited

Loading

stramel commented Sep 20, 2019 •

edited

Loading

stramel commented Sep 20, 2019 •

edited

Loading

8enSmith commented Sep 20, 2019 •

edited

Loading

8enSmith commented Sep 20, 2019 •

edited

Loading