Skip to content

typescript port of the pagination example #1534

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

Closed
wants to merge 1 commit into from

Conversation

ggascoigne
Copy link
Contributor

This is a direct port of the pagination example to typescript. It demonstrates how to extend the default types to reflect the plugins used by useTable. See ./src/types/react-table-local.d.ts. In this case, the usePagination plugin is used, and so those related types are merged with the defaults.

Note that there are some settings in this project that should change as soon as the final types are available, but they might be a hint to anyone using these types in ths short term.

  1. This package uses patch-package to removenode_modules/react-table/index.d.ts, since tsc was finding it despite my best efforts.
  2. The desired types have been copied into src/types/react-table.d.ts

Sadly I couldn't work out how to directly reference the index.d.ts that's in root from the example, tsc, really doesn't want to access files outside it's src root.

There are two bug fixes to the index.d.ts file - if people prefer I can submit them in a separate PR, and then update this to match later.

@ggascoigne ggascoigne mentioned this pull request Sep 21, 2019

module.exports = [
['use-babel-config', '.babelrc'],
// ['use-eslint-config', '.eslintrc'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this line crashed yarn start and I didn't have time to work out why.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 21, 2019

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 60d7fa8:

Sandbox Source
beautiful-chatterjee-q94fo Configuration

@tannerlinsley
Copy link
Collaborator

There have been changes to the API around useTableState. Please see the latest commits and release to get up to date.

@ggascoigne
Copy link
Contributor Author

note that this needs updating to support the new api changes.

@ggascoigne ggascoigne force-pushed the ts-pagination-example branch from ed47c31 to 9aee361 Compare October 10, 2019 18:18
@ggascoigne
Copy link
Contributor Author

I rebased my branch against master, hence the force-push, but it was cleaner than a big merge.

This PR has been cleaned up to support 7.0.0-beta.12, and is working correctly locally now.

This was referenced Oct 10, 2019
@Fieel
Copy link

Fieel commented Oct 15, 2019

@tannerlinsley nice, been trying to figure out what the problem was with the missing export of useTableState but I guess it's intentional

@ggascoigne ggascoigne force-pushed the ts-pagination-example branch from 95817de to 88c8fa2 Compare November 9, 2019 02:23
@ggascoigne
Copy link
Contributor Author

@tannerlinsley is there any way to re-run these tests, I didn't see anything in the logs to suggest that they failed because of anything in the changeset.

@tannerlinsley
Copy link
Collaborator

Make a commit to this PR with a comment or line-break. Should trigger the build again.

@ggascoigne
Copy link
Contributor Author

ah, I'd assumed that prettier would remove such a change.

@ggascoigne ggascoigne force-pushed the ts-pagination-example branch from 88c8fa2 to d3bcb27 Compare November 20, 2019 18:41
@ggascoigne
Copy link
Contributor Author

I just rebased on the latest master to see how that helps.

That said, I did notice that the ci:tests fail for me on master (latest). I get several errors like this:

 FAIL   unit  src/plugin-hooks/tests/useSortBy.test.js
  ● Test suite failed to run

    /Users/ggp/dev/tw-git/react-table/configs/tests/setup.common.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import '@testing-library/jest-dom/extend-expect';
                                                                                             ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
          at Array.forEach (<anonymous>)

 FAIL   unit  src/hooks/tests/useTable.test.js
  ● Test suite failed to run

    /Users/ggp/dev/tw-git/react-table/configs/tests/setup.common.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import '@testing-library/jest-dom/extend-expect';
                                                                                             ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
          at Array.forEach (<anonymous>)

@ggascoigne ggascoigne force-pushed the ts-pagination-example branch from d3bcb27 to 77708b7 Compare November 23, 2019 23:46
 This is a direct port of the pagination example to typescript. It demonstrates
 how to extend the default types to reflect the plugins used by useTable. See
 `./src/types/react-table-config.d.ts`. In this case, the usePagination plugin is
 used, and so those related types are merged with the defaults.

The project is very similar to the ./examples/pagination one, comparing
the two trees gives a very clear idea of how the two differ.
@ggascoigne ggascoigne force-pushed the ts-pagination-example branch from 77708b7 to dfe0f90 Compare November 24, 2019 00:02
@ggascoigne
Copy link
Contributor Author

cleaned up the history and rebased on the latest version of the library.

@owlyowl
Copy link

owlyowl commented Dec 2, 2019

Hi when trying to import from react-table in typescript I'm getting "Attempted import error: 'useFilters' is not exported from 'react-table'."

Just wondering if you want me to fix this?

@tannerlinsley
Copy link
Collaborator

I'm currently planning a restructure of the API that will be easier to add types for hopefully. Also, types are being moved out of this repo and into Definitely Typed.

Closing this for now. Can reopen if needed.

@ggascoigne
Copy link
Contributor Author

I'll admit that I'm really curious about any api restructuring that might make adding types easier.

@tannerlinsley
Copy link
Collaborator

It's mostly going to be centered around an effort to allow any relevant types to be tied to the plugins themselves, instead of having to be extended manually for every table component you build. Also, part of what I'm realizing here is that types are never going to be plug-and-play perfect around React Table unless it's fully written in Typescript. Even then, it would take a massive amount of restructuring to satisfy the types while also providing such a flexible API that allows for the things we're doing.

All I'm saying is, I'm investigating it.

@ggascoigne
Copy link
Contributor Author

BTW given the current (very desirable) feature set, I'm not sure that writing it in typescript would actually help with the issue either. The extensible nature of the api, pretty much inherently leads to self referential definitions.

@stramel
Copy link
Contributor

stramel commented Dec 2, 2019

So I have felt some burnout on trying to keep up with the definitions with lots of rework being thrown in and no chance to get the types updated before the release. Essentially by having them in the repo, it should be part of the release process to update the types before a release is cut.

I second @ggascoigne, and think that without some major API changes, that converting to typescript/coming up with a good external typescript definition, will be very difficult.

Personally, I think the easiest solution would be to rewrite it with typescript. This would allow us to see pain points as they come up and rework the API to allow nice typings.

@tannerlinsley
Copy link
Collaborator

Let's continue talking here #1591

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants