React-table: Project standards and conventions

[larrybotha]

Hey @tannerlinsley, loving react-table and would love to contribute and make things easier for you and contributors, so I'd like to open a discussion and see if you're happy to work with a checklist of standards and conventions to introduce.

Todo

Tasks are ordered by ease of implementation / impact on maintenance and reliability.

1. Code formatting
   
   
   
   1. [x] 1. add prettierrc.js to normalise prettier configs across all contributors setup
   
   
   2. [x] 2. use husky to automate formatting of code for contributors not using prettier without having to install prettier
   
   
2. Testing
   
   
   
   1. [x] 1. add custom jest.config.js for more flexibility over test configs (remove react-scripts dep?)
   
   
   2. [x] 2. add @testing-library/react for awesome testing
   
   
   3. [x] 3. add separate jest projects for unit testing and linting, as well as jest-watch-select-projects for switching between watched tests quickly
   
   
   4. [x] 4. add jest-watch-typeahead for faster selection of test files to run
   
   
   5. [x] 5. add is-ci-cli so that a single npm test script can be run for local _watched_ testing while CI runs through tests once
   
   
3. Commit message formatting
   
   
   
   1. [x] add commitizen to introduce a standard for commit messages to allow for automated semantic releases and generated changelogs
   
   
4. Changelog
   
   
   
   1. [ ] 1. generate changelog using conventional-changelog to save time, automate linking to issues from commitizen-formatted commits
   
   
5. Semantic releases
   
   
   
   1. [ ] 1. add semantic-release and configure Travis to automate semantic versioning based on commit messages. Ensure latest commit is tagged before installing.
   
   
6. Code coverage
   
   
   
   1. [ ] 1. update Travis to send code coverage results to codecov.io or similar on successful builds
   
   
7. TypeScript
   1. rewrite code in TypeScript, or (at least)
   
   
   
   1. [ ] add TypeScript module declarations
   
   

Let me know what you think about each point; will be happy to get working on the list as soon as you're happy!

[tannerlinsley]

I agree full-heartedly with every single point, with the exception of rewriting in typescript. Long story short, I'm not ready to full full typescript on open source code yet, but fully support having definitions.

[larrybotha]

@tannerlinsley understood - I'll keep the 2nd TS point in there as it's useful for consumers of react-table.

Will update the list, and get cracking on a couple :)

[nmccready]

One thing that makes the barrier of entry into typescript a whole lot easier and friendly is using Babel to compile it instead. It's much more forgiving.
https://babeljs.io/docs/en/babel-preset-typescript

[tannerlinsley]

@nmccready, Yes. When we bring in TS, we'll probably compile it using babel.

[tannerlinsley]

@larrybotha, I've set up some failing tests in master. Not sure why they aren't running correctly. Any ideas?

[larrybotha]

@tannerlinsley will have a look and get back to you :)

[larrybotha]

PR here: https://github.com/tannerlinsley/react-table/pull/1417

Required a few small tweaks to babel configs and versions. You'll see useTable passes now on all but one test.

I noticed you've got commit: "git commit" in packages.json. No biggie, but git-cz makes writing conventional commits super easy, but that's up to you :)

I'm using $ $(npm bin)/git-cz directly for the commitizen prompts.

[tannerlinsley]

Awesome!

I absolutely want to use git-cz. Still just trying to figure out the workflow we were talking about in #1410, how to enforce it, etc.

[larrybotha]

Want me to tackle enforcing the commit convention?

[tannerlinsley]

I would love that :)

[larrybotha]

Great, will get on it this evening :)

[tannerlinsley]

How can we generate the changelog and do the rest of these awesome things you have on this issue?

[larrybotha]

@tannerlinsley will get on this this evening. I need to do some evaluation on how alpha releases work with commitizen-changelog and semantic-release. I imagine the complexity has been accounted for, but I'll let you know

[tannerlinsley]

Awesome. Thanks for your help man. If I can do anything to help, let me know.

[larrybotha]

Only a pleasure; I'll let you know.

Wow, you're tearing through those issues and PRs like a monster - 2 issues (this included), and 0 PRs outstanding!

[paolostyle]

I could help with TypeScript typings - I already have some, because my whole project is written in TypeScript, though they're rather low effort for now (way too many anys); however I think it would be best to wait until at least beta, currently a lot of stuff is changing rather dynamically. I'm also wondering whether they should be in this repo or rather in DefinitelyTyped, generally it's recommended to only have type definitions in library's repo if it's written in TypeScript or if the types are generated automatically (probably based on JSDocs or something like this? not sure). So it would probably make sense to have it on DefinitelyTyped.

[larrybotha]

@paolostyle I agree; waiting for beta is a good idea. I'm not sure about where is best to host the definitions. I suppose there are a few things to consider, depending on the route taken.

Managed here

• pros
  
  
  
  • types and source are managed in a single repo
    
    
    
    
    
    • easier to maintain
    
    
    
  
  
  • users install a single module for both the library and definitions
  
  
  • _may_ be able to benefit from generating a declaration file
    
    
    
    
    
    • perhaps start with dts-gen
    
    
    
    • generate declaration file from JsDoc comments using tsd-jsdoc
    
    
    
  
  
• cons
  
  
  
  • generating a declaration file may result in incorrect declaration files
  
  

Managed on DefinitelyTyped

• pros
  
  
  
  • follows the recommended approach (which benefits make it the recommended approach? is it pragmatic to follow the recommendation, or is it idealistic?)
  
  
• cons
  
  
  
  • declaration file is in a separate repo
    
    
    
    
    
    • someone needs to remember that there is another repo that needs updating
    
    
    
    • additional work required to work in multiple repos
    
    
    
  
  

Issues common to both strategies

• ensuring types do not become stale (I've resorted to monkey-patching libraries that are written in JS that have stale declaration files; not a great experience for users)

I haven't looked into it deeply to determine what other strategies there may be to make this as painless to maintain as possible, while providing the best UX for users, so there may well be better or more reliable tools than what I've listed here.

Alternatively, a smaller benefit with lower overhead may be to use JsDoc and suggest TypeScript users enable checkJs, as per this article. I'm not sure if this works for editors other than VSCode (I don't use VSCode), or if it'll introduce issues with other js libs that have incorrect JsDoc declarations, however.

[tannerlinsley]

Just had a discussion about this here: https://spectrum.chat/react-table/general/the-types-for-typescript-are-outdated~79730c66-b5f8-48f0-b8d9-a187c3d2f435

TLDR, I would like the types to be kept over at DefinitelyTyped and distributed via @types/react-table. Then all we need to do is add that as a dependency in React Table and we're good to go.

My reasoning: https://blog.johnnyreilly.com/2019/08/symbiotic-definitely-typed.html, someday RT will be TS, but tomorrow is likely not that day.

[m-rutter]

On the topic of typescript declarations, there are a handful of github gists out in the wild attempting to provide type declarations for v7, such as this: https://gist.github.com/Grsmto/1ac3a7ddb8ad8288660784a37bff1798 The usefulness of these attempts is limited because of the use of the any type.

I might try writing my own declaration file in my current project that attempts to do better about inferring the types with generics, but it might help if the WIP v7 documentation for the public API were a little more fine grained in terms of the function types.

EDIT:
I'm guessing this will come with the first beta once the public API stabilizes.

[tannerlinsley]

I think this can be closed now as long as we track the rest of the items in their own issues like #1467