feat: add type declarations by jamesonhill · Pull Request #193 · Autodesk/react-base-table

jamesonhill · 2020-07-10T16:34:51Z

Initial pass at adding type declarations for each component's props as well as typings used by various event handlers. Feel free to comment or modify.

kachkaev · 2020-07-10T18:01:37Z

For the types to be picked by TS, package.json will need this extra line:

   "main": "lib/index.js",
   "module": "es/index.js",
+  "types": "dist/index.d.ts",

Relevant PR: #86

If it helps, here are some typings we've got in one of our projects. It's a very modest starting point, but free to reuse any this code in your diff 🙂

declarations/react-base-table.d.ts

declare module "react-base-table" {
  export interface RowExpandEvent {
    expanded: boolean
    rowData: {
      id: string
      parentId?: string
    } & { [key: string]: any }
    rowIndex: number
    rowKey: string
  }
  export interface ExpandIconBaseProps {
    expandable: boolean
    expanded: boolean
    depth?: number
    onExpand: (expanded: boolean) => void
  }
  export const unflatten: any

  const Table: any

  export default Table

  export interface BaseColumnInfo {
    key: string
    width: number
  }

  export interface FrozenColumnInfo extends BaseColumnInfo {
    frozen: true
    label: string
  }
}

types/index.d.ts

nihgwu · 2020-07-13T15:48:37Z

types/index.d.ts

+    /**
+     * The key field of each data item
+     */
+    rowKey?: string | number;


type RowKey = string | number?

isn't rowKey optional?

I think @nihgwu is suggesting to create a new separate type and then use it here as

rowKey?: RowKey

ah ok yea i can do that.

nihgwu · 2020-07-13T16:29:07Z

types/index.d.ts

+    /**
+     * Custom renderer on top of the table component
+     */
+    overlayRenderer?: React.ReactNode | (() => React.ReactNode);


maybe we could have a type CallOrReturn<T, P = {}> = T | ((p?: P, ...rest: any[]) => T)? I'm learning TS, not sure if thats the correct way

looks like this type is better type CallOrReturn<T, P = any[]> = T | (P extends any[] ? ((...p: P) => T) : ((p: P) => T))

@jamesonhill WDYT or it's not intuitive?

It can definitely make some of the typings cleaner. It would be used for any props where the current typings are someType | () => someType ?

yes, and more, like CallOrReturn<React.ReactNode> and CallOrReturn<React.ReactNode, Args>

I think i covered all of those types of instances in the changes i just pushed.

types/index.d.ts

nihgwu · 2020-07-13T16:33:37Z

types/index.d.ts

+     * A callback function for the header cell click event
+     * The handler is of the shape of `({ column, key, order }) => *`
+     */
+    onColumnSort?: ({ column, key, order }: SortByParams) => void;


I think we should separate it from sortBy

For compatibility reasons?

I'm thinking maybe something like

onColumnSort?: ({ column, key, order }): Pick<SortByParams, 'key' | 'order'> & { column: Column & { [key: string]: unknown } }

I mean { key: string; order: SortOrder; column: ColumnProps }

types/index.d.ts

package.json

types/index.d.ts

jamesonhill · 2020-07-15T12:51:57Z

I noticed in one of my repo's that there are multiple Column exports. Any suggestions on renaming the interface?

kachkaev · 2020-07-15T13:04:35Z

types/index.d.ts

+
+  type CallOrReturn<T, P = any[]> = T | (P extends any[] ? ((...p: P) => T) : ((p: P) => T));
+
+  export interface Column<T = unknown> {


How about ColumnConfig for this interface to avoid clashing with <Column />? @nihgwu WDYT?

It could be ColumnData but I'm personally not a big fan of data in interfaces because of how this term behaves when we need plurals:

type MyConfig = {/* */} myConfig: MyConfig = {} myConfigs: MyConfig[] = [] type MyConfigs = MyConfig[] myConfigs: MyConfigs = []

vs

type MyData = {/* */} myData: MyData = {} myDatas: MyData[] = [] // 😬 type MyDatas = MyData[] // 😬 myDatas: MyDatas = [] // 😬

Sorry I'm a newbie for TS, what about just IColumn or ColumnBase here? Sorry I still don't understand we have both Coumn and ColumnProps types here
Besides that, we allow arbitrary data in the column definition to solve the closure problem, maybe we should add [key: string]: any to the interface

All of the component interfaces that have a column or columns prop (and there are quite a few) share the same type, which is Column here. The <Column /> component also has same props but some additional as well. So by having Column or IColumn that interface can be re-used across all column or columns instances. And by extending Column in ColumnProps there is a single source of truth for what is a "column". Alternative being to duplicating it many many times which isn't ideal.

column and Column should share exactly the same props, we will extract Column's props to column, Column is actually just a sugar syntax

Maybe ColumnBase or BaseColumnProps ?

nihgwu · 2020-07-15T14:04:55Z

types/index.d.ts

+
+  type CallOrReturn<T, P = any[]> = T | (P extends any[] ? ((...p: P) => T) : ((p: P) => T));
+
+  export interface Column<T = unknown> {


Sorry I'm a newbie for TS, what about just IColumn or ColumnBase here? Sorry I still don't understand we have both Coumn and ColumnProps types here
Besides that, we allow arbitrary data in the column definition to solve the closure problem, maybe we should add [key: string]: any to the interface

nihgwu · 2020-07-15T14:09:53Z

types/index.d.ts

+     * Custom column cell renderer
+     * The renderer receives props `{ cellData, columns, column, columnIndex, rowData, rowIndex, container, isScrolling }`
+     */
+    cellRenderer?: CallOrReturn<React.ReactNode, {


for renderer we support element and elementType(function and class component), does this type support class component? like cellRenderer: ClassComponent

hmm, i'm not sure but my inclination is that it does not. React.ReactNode is very generic but i think it only covers the returned element types. So it can be ReactElement | ReactFragment | ReactPortal | string | number etc... I'll double check though.

nihgwu · 2020-07-15T14:18:37Z

types/index.d.ts

+
+  type CallOrReturn<T, P = any[]> = T | (P extends any[] ? ((...p: P) => T) : ((p: P) => T));
+
+  export interface Column<T = unknown> {


Can we just remove the T from Column? that would make the types clearer, I don't think the rowData should be constrained with a certain type, the only restriction is the unique id
Please correct me is I'm wrong, I'm still learning 😄

It's useful to have rowData typed, see Ant'd <Select> with a similar generic typing for value. Works like a charm for restricting wrong options and ensuring the onChange callback has the right typing too 👍

Each table instance would be able to set its own type and thus all all relevant props will be type-safe. For example, you won't be able to assign a callback that has a different expectation about the data.

nihgwu · 2020-07-15T14:22:03Z

types/index.d.ts

+    /**
+     * The sort state for the table, will be ignored if `sortState` is set
+     */
+    sortBy?: SortByParams;


{ key: string; order: SortOrder }

nihgwu · 2020-07-15T14:24:06Z

types/index.d.ts

+     * A callback function for the header cell click event
+     * The handler is of the shape of `({ column, key, order }) => *`
+     */
+    onColumnSort?: ({ column, key, order }: SortByParams) => void;


I mean { key: string; order: SortOrder; column: ColumnProps }

nihgwu · 2020-07-15T14:26:06Z

types/index.d.ts

+  }
+
+  export interface TableComponents {
+    TableCell?: CallOrReturn<React.ReactNode, {


they should be all elementType

nihgwu · 2020-07-15T14:27:20Z

types/index.d.ts

+    onExpand: (expanded: boolean) => void
+  }
+  export const unflatten: any;
+  export interface BaseColumnInfo {


what is this for?

These are the types @kachkaev posted at the top of the thread that he said were used in your repo typings.

I don't think those types should live in this repo

Removed them

nihgwu · 2020-07-15T14:27:34Z

types/index.d.ts

+    width: number
+  }
+
+  export interface FrozenColumnInfo extends BaseColumnInfo {


what is this for?

from here: #193 (comment)

If the only distinction between frozen and normal column infos is frozen?: true | "left" | "right" could be worth having just one column info indeed 🤔

nihgwu · 2020-07-15T14:27:56Z

types/index.d.ts

+
+  export interface FrozenColumnInfo extends BaseColumnInfo {
+    frozen: true
+    label: string


what's this?

from here: #193 (comment)

Hmm this could be our app-specific code got blended into the library type definitions. Thanks for spotting @nihgwu !

nihgwu · 2020-07-15T14:28:38Z

types/index.d.ts

+  }
+
+  export interface FrozenColumnInfo extends BaseColumnInfo {
+    frozen: true


for frozen column it could be true | 'left' | 'right'

nihgwu · 2020-08-09T15:40:42Z

@jamesonhill @kachkaev Sorry for the late response, I'm pretty busy recently, I pushed my changes based on my local test demo, feedbacks are welcome

jamesonhill · 2020-08-10T00:12:12Z

types/index.d.ts

+     * Whether the column is hidden
     */
-    className?: string;
+    hidden?: bool;


nihgwu · 2020-08-12T03:47:01Z

any inputs here, we still need to add types for the utils, or we can address it in another PR

jamesonhill · 2020-08-12T12:17:54Z

@nihgwu Everything that we have so far looks good to me. From what I can see, we're missing types for TableRow and TableHeader in addition to the utils. I think if we can at least get all of the components / props types in this PR that will be good. I can work on it some tonight. Any util types we can add here would be a bonus IMO.

nihgwu · 2020-08-12T12:23:10Z

@jamesonhill I don't know why I exported TableRow and TableHeader before which should not, so I think I'm OK without them, but the utils are useful, especially the unflatten for tree view

jamesonhill · 2020-08-12T12:31:23Z

Gotcha, well then in that case i'll just focus on the utils.

nihgwu · 2020-08-14T03:47:11Z

@jamesonhill I’m planning to release this with #213 , so I did some work on the typings for utils last night

nihgwu · 2020-08-14T06:53:52Z

a lot of codes are copied from @sskiswani 's work in #93, great thanks

jamesonhill · 2020-08-14T14:30:47Z

types/index.d.ts

+
+  export function normalizeColumns(elements: React.ReactNode[]): ColumnShape<any>[];
+
+  export function isObjectEqual(objA: any, objB: any, ignoreFunction?: boolean): boolean;


why not type objA and objB as object?

nihgwu · 2020-08-15T02:29:51Z

let's move forward and hear feedbacks to improve the types

nihgwu · 2020-08-16T10:39:50Z

@jamesonhill I got those error messages An index signature parameter type cannot be a union type. Consider using a mapped object type instead. for the use of [key: React.Key]: SortOrder; and expandedDepthMap: { [key: RowKey]: number }; and depthMap?: { [key: RowKey]: number }, in the console, any idea to fix them?

UPDATE: seems this commit could fix this issue 631d1bc

jamesonhill · 2020-08-16T23:38:56Z

@nihgwu It's not wrong, TS just doesn't support union types for index signature types (at least last I checked). Using in is the correct way to work around it.

jamesonhill · 2020-08-16T23:42:05Z

package.json

    "@babel/plugin-transform-runtime": "^7.0.0",
    "@babel/preset-env": "^7.0.0",
    "@babel/preset-react": "^7.0.0",
+    "@types/react": "^16.9.46",


This lib uses react 16.4. Are the typings for 16.9 consistent w/ 16.4?

I think that should be OK as it's dev only to suppress the warning about the types from React

jamesonhill · 2020-08-16T23:47:48Z

types/index.d.ts

+    minWidth?: number;
+    /**
+     * Whether the column is frozen and what's the frozen side
+     * Could be changed if we decide to allow Frozen.RIGHT


This comment somewhat confuses me. Frozen right is already supported, or am I misunderstanding?

Add initial typings

625842c

kachkaev reviewed Jul 10, 2020

View reviewed changes

types/index.d.ts Outdated Show resolved Hide resolved

jamesonhill added 3 commits July 10, 2020 18:37

Add types path

6ad6853

Update types

f081a0b

Add more row & column types

ae1c43b

nihgwu reviewed Jul 13, 2020

View reviewed changes

kachkaev reviewed Jul 13, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

jamesonhill added 6 commits July 13, 2020 13:58

Fix types path

f858e7e

Add generic params

4ae4651

Add missing type param

8f17cf0

Update cellRenderer props

e1f4fe9

Remove omit

b4847ed

Add callOrReturn type

5f4467e

kachkaev reviewed Jul 14, 2020

View reviewed changes

types/index.d.ts Outdated Show resolved Hide resolved

kachkaev reviewed Jul 14, 2020

View reviewed changes

types/index.d.ts Outdated Show resolved Hide resolved

Update types

69e1725

kachkaev reviewed Jul 15, 2020

View reviewed changes

nihgwu reviewed Jul 15, 2020

View reviewed changes

jamesonhill and others added 2 commits July 15, 2020 10:43

Remove types

d4e0ca7

update types from Neo

e141a3f

jamesonhill commented Aug 10, 2020

View reviewed changes

types/index.d.ts Outdated

* Whether the column is hidden

*/

className?: string;

hidden?: bool;

Copy link

Contributor Author

jamesonhill Aug 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

nihgwu added 2 commits August 10, 2020 12:02

fix typo

971b607

fix column consts

ba2ffe4

jamesonhill and others added 3 commits August 13, 2020 20:12

Add some util types

0f04bf7

Fix normalizeColumns arg type

26aaafc

add types for utils from Neo

04f4a10

nihgwu added 3 commits August 14, 2020 12:53

update

726adc7

fix unflattern return type

76af13d

add types for methods

1b0d5f6

jamesonhill commented Aug 14, 2020

View reviewed changes

update

a080f55

Merge branch 'master' into ts-type-declarations

36b32e4

nihgwu changed the title ~~WIP: Add type declarations~~ feat: add type declarations Aug 16, 2020

nihgwu added 2 commits August 16, 2020 16:09

changelog

397bf30

fix

5b81efa

fix

631d1bc

nihgwu force-pushed the ts-type-declarations branch from 01eae47 to 631d1bc Compare August 16, 2020 10:54

jamesonhill commented Aug 16, 2020

View reviewed changes

update

5a5adde

nihgwu merged commit 4231399 into Autodesk:master Aug 17, 2020

nihgwu deleted the ts-type-declarations branch August 17, 2020 02:25


		type CallOrReturn<T, P = any[]> = T \| (P extends any[] ? ((...p: P) => T) : ((p: P) => T));

		export interface Column<T = unknown> {


		export function normalizeColumns(elements: React.ReactNode[]): ColumnShape<any>[];

		export function isObjectEqual(objA: any, objB: any, ignoreFunction?: boolean): boolean;

Conversation

jamesonhill commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kachkaev commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nihgwu Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jamesonhill commented Jul 15, 2020

Uh oh!

kachkaev Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nihgwu Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesonhill commented Jul 10, 2020 •

edited

Loading

kachkaev commented Jul 10, 2020 •

edited

Loading

nihgwu Jul 14, 2020 •

edited

Loading

kachkaev Jul 15, 2020 •

edited

Loading

nihgwu Jul 15, 2020 •

edited

Loading