Improve Table typing

[r100-stack]

See migration guide (section) to see how this PR affects you.

---

Changes

• We no longer depend on @types/react-table. Instead, I merged the types from @types/react-table with our custom types from react-table.config.ts. The new file is called react-table.ts.
  • We now export all react-table types from a new endpoint called /react-table.

    • import { Table } from '@itwin/itwinui-react';
      - import { Column, TableState } from 'react-table';
      + import { Column, TableState } from '@itwin/itwinui-react/react-table';

    • But for our internal files, we make no change. i.e. We still import from react-table. This is because of the following reason:
      • Since we don't depend on @types/react-table anymore, I added paths to the compilerOptions in itwinui-react/tsconfig.json to point to our react-table-types.d.ts for the types. This enables intellisense/type checking to work correctly without any import change.
• Exposed endpoint /react-table works only for new node versions. Thus, react-table.ts is transpiled to react-table.d.ts and stored in the root folder so that this file is accessible to users with old node versions who cannot use the exports map in package.json and have to import by paths. (Improve Table typing #1514 (comment))
• Instead of each of the row data to extend object (like in @types/react-table), each row data now extends Record<string, unknown> (like in our Table files (e.g. TextFilter))

  • - export type Column<D extends object = {}> = ...
    + export type Column<D extends Record<string, unknown> = {}> = ...

• Since we are now fixing Table types properly, I undid the parts of Table: allow passing top-level Header again #1072 that were meant to be temporary changes.
• Created tsconfig.react-table.json to transpile react-table.ts to react-table.d.ts. This is used in @itwinui-react's build and dev:types scripts.
• @itwinui-react's test is not split into yarn test:types (new) and yarn test:unit (existing). yarn test:types checks the types of all Table files (including unit tests, etc.) and not just the files that will be built using tsc.

Testing

First, I ensured that the below long and convoluted type error when doing [...] satisfies Column<DataType>[] was fixed:

...

Type Record<string, unknown> is not assignable to type TableStoryDataType.

• Then I created react-table-types.test.tsx to only verify that there were no type errors for some unique cases. This also ensures there are type errors where we expected them. No unit testing is done here.
• I then fixed all type errors in Table.story.tsx.
• I created yarn test:types scripts that ran tsc --noEmit (docs) on all Table files in itwinui-react (including react-table-types.test.tsx), and also Table.stories.tsx in storybook.
  • As mentioned in the below TODOs section, I hope to extend these type checks to other files too in a future PR.

Docs

I fixed all type errors in Table.stories.tsx. These fixes demonstrate how to use satisfies or sometimes provide types to the function parameters to solve type errors.

I did not add anything to the website, because the Table doc page was not complete before this PR. When working on the Table page in the future, we can include info about this PR.

I updated CONTRIBUTING.md to mention that itwinui-react unit tests are now run using yarn test:unit instead of yarn test. I didn't mention about yarn test:types as of now since this script only tests the types for Table files and not all files. But in the future PR that checks the types for all files, I will include info about this script.

TODOs:

• This PR:

  • Confirm that the Table -> Full story's hover is working correctly.
  • Confirm that the Table -> StatusAndCellIcons story is loading correctly.
  • Double check react-table-types.d.ts once again (Improve Table typing #1514 (comment))

• Separate PRs:

  • Solve type errors after 7b4c2c8. For now I reverted this commit. I will solve these type problems in another PR.
  • Disallow top level header. i.e. Remove the backward compatibility added in Table: allow passing top-level Header again #1072
  • Test for types across all pertinent parts of the repo, instead of just type checking in Table files (Maintenance #1571)
  • Add Table types info to docs site

[mayank99]

can we make some of these less verbose by doing this?

Suggested change

	) satisfies NonNullable<
	TableProps<{
	name: string;
	description: string;
	}>['onExpand']
	>;
	) satisfies NonNullable<TableProps['onExpand']>;

if the user needs additional type checking, they can add the extra data types, but shouldn't be required to.

[mayank99]

i also noticed that state is any for some reason

[r100-stack]

can we make some of these less verbose by doing this?

234f4f8: I removed all such satisfies NonNullable<...> that used { name: string; description; }. But I didn't remove the ones that already use TableStoryDataType or something similar because they were not too verbose. That way we can show some variations while still not being verbose.

[r100-stack]

i also noticed that state is any for some reason

Looks like this is happening when state is assigned a type from react-table-types.ts itself.

Because when I change it to some primitive type like string or number, the type of state is inferred correctly.

I haven't been able to find a fix so far, but just wanted to give a quick update while I am still searching for the fix.

[r100-stack]

I realized that this was happening because in Table.tsx we were importing react-table, instead of importing by path.

E.g. in Table.d.ts generated by tsc using Table.tsx

- import type { CellProps, TableOptions, Row, TableState } from 'react-table';
+ import type { CellProps, TableOptions, Row, TableState } from../../react-table/react-table.js

Importing from react-table from this file imports from the real react-table package instead of from the path mentioned in paths of tsconfig.

I did this commit (7b4c2c8) to fix the above problem. I noticed quite a few type errors in Table.stories.tsx. To avoid convoluting this PR, I reverted that commit and will fix the above issue in another PR.

[mayank99]

fine by me

[mayank99]

LGTM! nice work, and everything seems to work much better now.

would be nice if you can remove outdated parts from PR description before merging.

[r100-stack]

LGTM! nice work, and everything seems to work much better now.

Thanks!

would be nice if you can remove outdated parts from PR description before merging.

Sounds good, updated.

[gretanausedaite]

No further comments from my side. LGTM 🌵

Edit: do we need wiki for these changes?

[r100-stack]

do we need wiki for these changes?

Added to the v3 migration guide: