-
Notifications
You must be signed in to change notification settings - Fork 39
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
Improve Table typing #1514
Merged
Merged
Improve Table typing #1514
Changes from 211 commits
Commits
Show all changes
217 commits
Select commit
Hold shift + click to select a range
a1c1f80
Initial commit
xman343 f0a25cf
Merge branch 'dev' into xander-datepicker-a11y-fixes
xman343 a11c4b8
Merge branch 'dev' into xander-datepicker-a11y-fixes
xman343 23fc9bb
useId added to label date table by current month
xman343 d63332f
aria label added to DatePicker button in Menu ex
xman343 ee7425e
changeset
xman343 9c9f1d0
variable created for useId()
xman343 4a1712c
changeset update
xman343 4e60dd9
menu ex updated
xman343 4510505
menu ex label update
xman343 0fccd0e
Merge branch 'dev' into xander-datepicker-a11y-fixes
xman343 10be798
Merge branch 'dev' into xander-datepicker-a11y-fixes
xman343 7277515
Partially edited index.d.ts. Playground table from last week + remove…
r100-stack 0db913c
Trying to add react-table package folder inside itwinui-react folder
r100-stack 179330f
WIP
r100-stack 482449b
WIP
r100-stack 602e357
Temp allowing 1 warning in eslint
r100-stack 26db866
Added react-table (backup) before removing the dep
r100-stack 9932a6a
Fixed all errors (need to confirm if those changes are logically corr…
r100-stack b8c01a8
Change dep to local folder for `@types/react-table` and removed `colu…
r100-stack aa2bf77
Combined Cell from react-table-config with index.d.ts
r100-stack e3d6746
Combined everything from react-table-config with index.d.ts
r100-stack 4ae39b5
WIP
r100-stack 86b48f9
WIP
r100-stack 2ff2b82
Working so far
r100-stack 9b2a8f1
Not working after adding the definition for Renderer
r100-stack 122c1f3
Working after removing other types
r100-stack 5ba1ee7
Working after readding all definitions but removing unnecessary prope…
r100-stack e7adb00
WIP
r100-stack 2f9235b
Stops working when both Filter and Cell are provided
r100-stack 4ebff72
WIP
r100-stack 22e1310
WIP
r100-stack 47b3f34
WIP
r100-stack bfb60c5
Pasted newly merged index.d.ts from the playground
r100-stack 538fd91
Wrapped index.d.ts code in declare module
r100-stack 1089ecc
Deleted some comments and code from App.tsx
r100-stack fe5a45c
Commented out old-not-working-index.d.ts
r100-stack f250ba8
Removed dep on types/react-table
r100-stack 83137c7
Fully commented out react-table-config.ts
r100-stack 5fcc72c
Trying to import react-table types into Table.tsx
r100-stack df2fec3
WIP
r100-stack 2cfc940
Updated the imports from react-table to the react-table-types.js file…
r100-stack eacefb4
Small trial and error changes
r100-stack a276dcd
Trying moving the types folder within code
r100-stack 18d7aa5
Trying to move react-table-types.d.ts to src/core/Table/types
r100-stack 02014c5
Add postBuildTypes.mjs to copy react-table-types.d.ts to the cjs/esm …
r100-stack acfb8fd
Working export of types (export type) instead of just values (export)
r100-stack d9914ba
Proper exports along the barrel files
r100-stack 3eeb665
Changed some leftover imports from react-table to react-table-types.js
r100-stack 9bfee47
Removed unnecessarily testing files
r100-stack e9da2e5
Modified a few files to start using TableTypes instead of directly fr…
r100-stack 55b492c
Export TableProps
r100-stack 6d58e8f
Fixed some type errors of missing ids in Table data's subRows
r100-stack ae34d45
Merge branch 'dev' into rohan/table-types
r100-stack 6a58ad9
Added back all types on TableStoryDataType
r100-stack c2d1edf
Merge branch 'dev' into rohan/table-types
r100-stack a12ef70
Merge branch 'dev' into rohan/table-types
r100-stack b5ff12c
Changed all object to Record<string, unknown>
r100-stack 4201b30
Undid some old trial and error type changes
r100-stack 279f1be
Moved TableProps and TablePaginatorRendererProps from Table.tsx to re…
r100-stack 0bd10bd
Moved back TableProps and TablePaginatorRendererProps from react-tabl…
r100-stack 2557ea8
Fixed imports and undid unnecessary changes
r100-stack 2e3424b
Undid previous trial and error change
r100-stack 3600272
Changed flattenColumns to return columns itself since we don't suppor…
r100-stack 76cccaa
Removed two unnecessary type assertions
r100-stack 9749a71
Remove commented code
r100-stack 31df020
Temporarily created react-table-objects.ts
r100-stack 287551f
Deleted temp file react-table-objects
r100-stack 61590ae
Corrected leftover paths
r100-stack 034ef1d
Added paths to compilerOptions in tsconfig.json so that we can import…
r100-stack 23c37e1
Added comment explanation
r100-stack 3abee9f
Reformat file
r100-stack 10e75f9
Changed all imports of the types file to now using react-table
r100-stack 9521010
Removed the declare module wrapper to solve the "namespace does not e…
r100-stack 30b882f
Reformat file
r100-stack a13ed81
Few parts of the written debug code
r100-stack e2df9fe
Some type error fixes
r100-stack 26e9e4c
Some basic react table type tests
r100-stack 4c9bd22
Added @ ts-expect-error for the existing unit test that is giving typ…
r100-stack afbfd2f
Added multiple @ts-ignore to stop tsc reporting the unused variable n…
r100-stack 0e614aa
Added test:types to use tsconfig.test.json. Also edited the other pac…
r100-stack 4e9bb57
Removed some unnecessary variable names that were unused. Thus, also …
r100-stack 315a2d8
Fixed all type errors in Table.stories.tsx
r100-stack 6cae5bd
Removed unnecessary satisfies
r100-stack 00b6dc4
Exported DefaultCellProps and EditableCellProps
r100-stack 9ab12fa
Removed code duplication
r100-stack c4ad330
Removed unnecessary comments
r100-stack 45f54f7
Removed testing variables and unused imports
r100-stack d542969
Remove unnecessary comments
r100-stack 6397770
Helpful explanation comments
r100-stack affa6e1
Tested that Column and TableTypes.Column are equivalent
r100-stack 859d9e2
Removed unnecessary diff
r100-stack 00396bc
Combined two similar type tests
r100-stack 358f211
Removed unnecessary diffs
r100-stack c47954a
Delete old testing files
r100-stack eae6ee9
Removed unnecessary changes
r100-stack cc8ca64
Comment explanation
r100-stack f68aed0
Ignore react-table-types.test.tsx from jest since this file is only t…
r100-stack 8ab6cff
Typo
r100-stack b8cb713
Fixed type errors in Table.stories.tsx
r100-stack 12a99a0
Better structured code for test:types only for Table
r100-stack 718e56f
Renamed postBuildTypes to postBuild_Types to not confuse it with scri…
r100-stack c5a3171
Removed unnecessary file from tsconfig.build.json
r100-stack 732064b
Delete old testing files
r100-stack 13e6747
Reverted playground
r100-stack 24b09bb
Removed unnecessary `compilerOptions` from `tsconfig.json`
r100-stack 3c1352d
Changeset
r100-stack e16a1c0
Merge branch 'dev' into rohan/table-types
r100-stack c9f583b
Revert eslint change
r100-stack fbbad82
Revert root level test:types to minimize file changes in PR
r100-stack 174bf8e
Removed exporting of inner props
r100-stack 5e94e8c
Fix a type error in Table stories
r100-stack 67e8fcc
Reverted functionality change of the logic for flattenColumns. Added …
r100-stack 7f9d5f4
Re-export TableProps
r100-stack 8fa777d
Removed args
r100-stack 6bd9547
Merge branch 'rohan/table-storybook-remove-args' into rohan/table-types
r100-stack 45ca69b
Removed unnecessary type assertion
r100-stack a5bf4c7
Removed one more unnecessary type assertion
r100-stack 4a30eeb
Merge branch 'dev' into rohan/table-storybook-remove-args
r100-stack ee701d4
Merge remote-tracking branch 'origin/rohan/table-storybook-remove-arg…
r100-stack 9342581
Merge branch 'dev' into rohan/table-types
r100-stack 08d5b6f
Added copyright header to react-table-types.tsx
r100-stack f7140d1
Trying to use .ts instead of .d.ts. Used declare and commented export…
r100-stack 7bb6598
Testing change
r100-stack 4194d20
Undid testing change
r100-stack fc3cfb5
Replaced default top of file comments with custom comments attributin…
r100-stack 61137ea
Changed code divider comment's style
r100-stack 0f47a7a
Added few reasons for hard fork of @ types/react-table
r100-stack fd13f3a
Change storybook types "test" to "build"
r100-stack 8fe0b0f
Fixed type errors in Table.stories.tsx
r100-stack 9863f51
Working exposing table entry point
r100-stack 89bf4fe
Created a new endpoint called table
r100-stack 2b7e175
Removed TableProps export
r100-stack 2439952
Fixed Table storybook type errors
r100-stack 2d0a0e5
Small Table story type improvements
r100-stack 92c5474
Added a complex Table example
r100-stack 28b107e
Corrected export path
r100-stack 1980aed
Merge branch 'dev' into rohan/table-types
r100-stack facf20d
Removed incorrect export
r100-stack f262352
Removed unnecessary comment
r100-stack d2da820
Renamed /table entrypoint to /react-table
r100-stack 0b9f523
Added test to confirm custom Filters don't give type errors
r100-stack 2c31502
Revert playground
r100-stack de72f34
Leftover from rename /table to /react-table
r100-stack ddce4b5
Added the fallback react-table file
r100-stack 2e5f0a1
Temp
r100-stack 5d92efe
Deleted postBuild_types.mjs
r100-stack 22df9d6
Reverted module and moduleResolution to Node16/NodeNext
r100-stack 230b734
Revert excess change
r100-stack 6ccf450
Merge branch 'dev' into rohan/table-types
r100-stack d854e1f
Removed copyfiles dep
r100-stack e9c27d7
Revert yarn.lock
r100-stack 5a4fa69
Remove .stylelintrc.json leftover changes
r100-stack d3172f7
Remove .stylelintrc.json leftover changes
r100-stack 30acb3b
WIP - still not working
r100-stack a1b9ebb
Undid .swcrc change
r100-stack 54d0c48
Revert "WIP - still not working"
r100-stack a3edccd
Spacing for better text formatting
r100-stack 40a6b72
Fixed use of incorrect endpoint
r100-stack 809be55
Fixed reference to incorrect endpoint
r100-stack 7a043ac
Added a small test to confirm types can be imported by file path or p…
r100-stack cab7f9c
Added ./react-table.d.ts to package.json -> files
r100-stack 5bdfc9d
Mentioned "yarn test" to "yarn test:unit" in CONTRIBUTING.md
r100-stack 174baa3
Added TODO
r100-stack 60749e0
Build react-table types file in root folder using custom tsconfig. Re…
r100-stack a7f2c36
Removed unnecessary file
r100-stack d9830f5
Fixed few errors by fixing path
r100-stack 5d2d043
Added emitDeclarationOnly
r100-stack 523947f
Added react-table.d.ts to clean:build
r100-stack b33ef87
WIP testing
r100-stack e074fc5
Set module & moduleResolution to Node16 in tsconfigs. Added type: mod…
r100-stack 6a4236c
Fixed an error of required extension for relative imports
r100-stack 8bf2bbd
Workaround for emotion/styled not working in ESM
r100-stack 07784ca
Fixed errors in next-playground
r100-stack 2d95621
Merge branch 'rohan/playground-moduleResolution' into rohan/table-types
r100-stack 5d4d1d3
Removed module/moduleResolution in astro playground
r100-stack 06470ba
Reordering keys in package.json for clarity and uniformity
r100-stack efb929b
Changed moduleResolution from Node16 to Bundler
r100-stack a22d11c
Use moduleResolution=Node16 for next-playground since next may not su…
r100-stack 0086c29
Merge branch 'rohan/playground-moduleResolution' into rohan/table-types
r100-stack eedcc83
Removed react-table.js from package.json
r100-stack 7960276
Use file extensions when doing relative imports in the vite playground
r100-stack c380d88
Merge branch 'rohan/playground-moduleResolution' into rohan/table-types
r100-stack bc9f4c8
Updated changeset
r100-stack 10d159e
Reverted few unnecessary changes
r100-stack d13ddb7
Renamed react-table-types.test.tsx to react-table.types-test.tsx to a…
r100-stack ab9223c
Removed unnecessary default
r100-stack 3c953e9
Remove redundant --emitDeclarationsOnly
r100-stack ac68f3f
Move custom additions to the top
r100-stack 7c1ca72
Corrected the path of tsconfig tests
r100-stack e19be97
Merge branch 'dev' into rohan/table-types
r100-stack 25d7448
Merge branch 'rohan/table-types' of https://github.com/iTwin/iTwinUI …
r100-stack 21d1c0f
Added generic to `<Table>`
r100-stack 2f8cec6
Removed unnecessary type assertions
r100-stack cf815ff
Moved TableFilterProps to the custom additions section. Also moved al…
r100-stack 2cd4729
Added clarifying comment
r100-stack d0c2a6a
Changed all subsets of TableRoeDataType to TableRowDataType
r100-stack fac1269
Fixed missing dep in dep array warning
r100-stack c4d7d59
Removed unnecessary test
r100-stack b71a06d
Revert "Removed unnecessary test"
r100-stack 03243ad
Replaced all react-table types alias imports with individual imports …
r100-stack f7813ba
Leftover: Replaced all react-table types alias imports with individua…
r100-stack 94c3e54
Removed unnecessary test
r100-stack 280f183
Removed few unnecessary comments
r100-stack 50ea7b9
Added react-table (remade).ts as a temp file to confirm if no types a…
r100-stack 9655b76
Made a few changes to react-table.ts to be similar to react-table (re…
r100-stack f02bd8f
Deleted react-table (remade).ts
r100-stack 2a1a04d
Reverted playground
r100-stack 0007c27
Removed unnecessary generic type assignment
r100-stack 6bd30f6
Make Column used in file
r100-stack 33f9179
Reverse order of test:types and test:unit to pass --coverage in CI to…
r100-stack fdff0a9
Leftover changes of subset types to TableRowDataType
r100-stack 234f4f8
Removed a few verbose types in favor of less strict types
r100-stack 82881f6
Merge branch 'dev' into rohan/table-types
r100-stack 7b4c2c8
Removed paths from tsconfig since they were hiding TS errors. In prog…
r100-stack 6a76170
Revert "Removed paths from tsconfig since they were hiding TS errors.…
r100-stack 177c226
Merge branch 'dev' into rohan/table-types
r100-stack File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@itwin/itwinui-react': major | ||
--- | ||
|
||
Table now has better type support. Users must now import types from `@itwin/itwinui-react/react-table` instead of from `@types/react-table`. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
{ | ||
"$schema": "https://json.schemastore.org/tsconfig", | ||
"extends": "./tsconfig.json", | ||
|
||
// Temporarily testing only Table.stories.tsx. | ||
// TODO: Fix all tsc errors in all stories and then include all stories. | ||
"include": [], | ||
"files": ["src/Table.stories.tsx"], | ||
|
||
"compilerOptions": { | ||
"outDir": "dist" | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we make some of these less verbose by doing this?
if the user needs additional type checking, they can add the extra data types, but shouldn't be required to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i also noticed that
state
isany
for some reasonThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
234f4f8: I removed all such
satisfies NonNullable<...>
that used{ name: string; description; }
. But I didn't remove the ones that already useTableStoryDataType
or something similar because they were not too verbose. That way we can show some variations while still not being verbose.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is happening when
state
is assigned a type fromreact-table-types.ts
itself.Because when I change it to some primitive type like
string
ornumber
, 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that this was happening because in
Table.tsx
we were importingreact-table
, instead of importing by path.E.g. in
Table.d.ts
generated by tsc usingTable.tsx
Importing from
react-table
from this file imports from the realreact-table
package instead of from the path mentioned inpaths
oftsconfig
.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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine by me