React-hooks-testing-library: Convert to TypeScript

[mpeyper]

Describe the feature you'd like:

Title says it all. I feel like TypeScript has reached a level of adoption where we shouldn't be scared of losing contributors by using it instead of JavaScript.

I believe other @testing-library packages are considering or currently being converted, if they aren't already TS.

Suggested implementation:

We currently have type definitions in DefinitelyTyped which can be used as the basis of the types.

I think we can aim to have a straight cut over to a new codebase rather than a progressive migration as there are not that many files to consider.

I think we should look at using TSDX for the tooling unless anyone has other ideas or suggestions in this space.

Describe alternatives you've considered:

N/A

Teachability, Documentation, Adoption, Migration Strategy:

This package should also become the source of truth for the type definitions and the DefinitelyTyped version should be deprecated. I'm not sure what the process if to do that if there is one.

Other than that I don't think there are any particular considerations to be made for this.

[joshuaellis]

The compiler won't recommend looking for @types/* library anymore because we'll have bundled types. So I imagine they would be able to put a depreciated flag on the package?

Is there a reason you think we should tsdx? Looking at the repo, it's pretty immature and there's a few outstanding issues. I think we could probably just use our own configuration. Proabably easier to maintain overall.

[nickmccurdy]

I think we should look at using TSDX for the tooling unless anyone has other ideas or suggestions in this space.

kcd-scripts now supports TypeScript and it's already the de facto standard tool for this organization.

This package should also become the source of truth for the type definitions and the DefinitelyTyped version should be deprecated. I'm not sure what the process if to do that if there is one.

There's a list of removed packages that you can submit a pull request to.

[mpeyper]

Is there a reason you think we should tsdx?

Only that I have seen folks on Twitter mention how nice it was to use.

I'm also happy with kcd-scripts if that is the preferred approach.

[marcosvega91]

If you want I can migrate the actual codebase to kcd-scripts.

[nickmccurdy]

In that case it's probably easier to do #499 with yo kcd-oss first. We'll have better CI for this PR and TS tooling setup will be simpler.

[marcosvega91]

Yes, sure

[tigerabrodi]

@marcosvega91 @mpeyper I'd love to help when it comes to migrating to TypeScript :two_hearts:

[mpeyper]

@tigerabrodi

You can start building off the changes in #502 which I'll merge as soon as I'm on my laptop next (I need to update some repo settings first)

[tigerabrodi]

@mpeyper Do you mean building in the same PR as @marcosvega91? Then he would have to give me permission as a collaborator to be able to push changes there, or am I missing something?

I could start building once that PR is merged :raised_hands: :smiling_face_with_three_hearts:

Thanks for the opportunity also :pray: :two_hearts:

[mpeyper]

No, I mean pull down his PR branch, branch from there, and start committing onto it. Then rebase to master once I've merged it (within 24 hours).

[tigerabrodi]

@mpeyper How do I do that? I have forked render hooks and my current branch is master. How do I pull down his PR branch?

Thanks! :pray:

[mpeyper]

I'm only on my phone, so I'm a bit hamstrung to give you the exact commands, but first you need to add their repo as a remote, then you pull the changes from that remote's branch.

You can follow the links near the top of the PR page to work out the user and branch names involved.

[nickmccurdy]

You can also use the Open with button if you have GitHub Desktop or Codespaces, otherwise you can install gh and use gh pr checkout 502.

[tigerabrodi]

Will do it later, two tasks to cross off my side project, I am looking forward to it though haha :heart_eyes: :100:

Probably just install gh then do what @nickmccurdy said :tada:

Thank you guys, you are so awesome :smile:

[nickmccurdy]

502 has been merged, so at this point you can pull down master and branch off it directly.

[tigerabrodi]

Yeah lit :fire: :metal:, looking forward to it :smile:

[tigerabrodi]

@nickmccurdy @mpeyper @marcosvega91 I could create the first PR with a small change, or how do you want to have it?

[nickmccurdy]

This package has a relatively simple API so it might be easier to review in a single PR that converts everything, but it's up to you.

[tigerabrodi]

I was thinking of other's wanted to collaborate, how would we do it then? One option is to invite someone as a collaborator to my fork of render hooks right?

[nickmccurdy]

If you open a pull request, maintainers should be able to push to your fork automatically, and you can invite anyone else. If permissions become an issue we might be able to open a branch here, but let's keep it simple for now.

[tigerabrodi]

I will open a PR tomorrow if no one has opened it yet, if someone has, I will contribute tomorrow :tada:.

If you guys want, you can assign this issue to me, and I will open a PR tomorrow.

Thanks again for this opportunity :smiling_face_with_three_hearts:

[mpeyper]

@tigerabrodi the kcd-scripts, GitHub Actions and semantic release changes are in master now, in case you hadn't noticed yet.

Edit: I didn't notice this had already been mentioned in a previous comment... Carry on.

[tigerabrodi]

I am so excited :smiley:, I am sort of new to OSS, I think it will take some time for me to relax from all these energetic feelings, I am just way too happy haha :smile:

[tigerabrodi]

@mpeyper Is it a must that the PR should be opened tomorrow :grimacing:, I'd love to work on this with a couple of friends :smiling_face_with_three_hearts:, but we are currently finishing another PR. Is it okay to open it sometime next week :thinking:? Maybe next weekend :roll_eyes:?

Edit: I will open the PR tomorrow, sorry for all the confusion lol :smile:

[mpeyper]

Just for the record, we're not in any rush. Whenever it works for you, works for me.

Also, the PR can be opened with a single file changed, and you can iterate on it until it's ready. Often it's easier to get feedback early before you go too far down a particular path which makes adjusting less daunting down the track.

[tigerabrodi]

@marcosvega91 KCD Eslint rules seem to be wrestling with me :thinking:? Should I disable these rules or?

Image

[joshuaellis]

Potentially a silly thing, are you sure your eslint extension is using the project's configuration and not some sort of universal default?

[tigerabrodi]

@joshuaellis Hmm, the errors went gone when I removed the first ./ when extending eslint config from kcd scripts.

[joshuaellis]

@tigerabrodi are you using yarn or npm? I had issues with yarn earlier. But this issue I didn't have. I'm assuming though, the reason we have the relative path is incase you have more than one version of kcd-scripts installed in global or something? @marcosvega91 thoughts?

[marcosvega91]

I think that is like the issue with yarn. There is a conflict with eslint versions, we could add eslint to this project to avoid the problem.

[tigerabrodi]

I am not using yarn :+1:

[tigerabrodi]

I am gonna initialize TypeScript for this project now and open the PR with the very first small change :+1:, any tips on initializing TypeScript for this project? :roll_eyes:

[marcosvega91]

You should only add tsconfig.json in the root folder with

{
  "extends": "./node_modules/kcd-scripts/shared-tsconfig.json"
}

and then everything is ready to go!! Start renaming files and it should be great ✌️

[tigerabrodi]

@marcosvega91 :smiling_face_with_three_hearts:

[tigerabrodi]

@marcosvega91 Hold on :see_no_evil:

[marcosvega91]

try to restart typescript server. It should be CTRL + SHIFT + P and then type typescript server

[tigerabrodi]

Seems like it does take some time to initialize :see_no_evil:

[tigerabrodi]

@marcosvega91 I tried running npm i to see what would happen because it is not finishing initialization.

I get this error:

[tigerabrodi]

This seems to be wrong :thinking:?

error TS18003: No inputs were found in config file '/home/tigerabrodi/Desktop/open-source/react-hooks-testing-library/tsconfig.json'. Specified 'include' paths were '["node_modules/kcd-scripts/../../src/**/*"]' and 'exclude' paths were '["node_modules/kcd-scripts/node_modules"]'

[marcosvega91]

Yes this error is because you don't have file typescript yet. When you start renaming js files to ts you should not have the problem anymore

[tigerabrodi]

We will try creating a PR this week with a small change from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/testing-library__react-hooks :clap: :crossed_fingers: :smiling_face_with_three_hearts:

[Goldziher]

I can convert this library to TS, or is someone already doing this?

[tigerabrodi]

@juhanakristian Should open a PR today :+1:

cc @Goldziher

[mpeyper]

@Goldziher I think they're planning on doing the conversion fairly openly and collaboratively in #515. Perhaps you can jump on board and help them out? @tigerabrodi?

[tigerabrodi]

@Goldziher You are more than welcome :smiling_face_with_three_hearts: :hugs: :raised_hands:

@juhanakristian Has to invite you to his fork, just ping him confirming that you wanna jump on board :tada:

@Goldziher Not a Requirement :wink: Would be happy if you could join https://kentcdodds.com/discord/, we communicate there in a specific channel, I mean, if you wanna jump on board and help us out, it'd mean a lot :smiley: :partying_face: :smiling_face_with_three_hearts:

[Goldziher]

Will do 😃. Thanks for the invite.

[Goldziher]

@tigerabrodi , I joined the server. What channel Am i looking for (this is a huge server).

[joshuaellis]

Hello, Typescripters!
I'm working on #68 and one of the features we wanted to include was a form of auto-selecting the correct test renderer based on the user's devDependencies. I'm having a bit of difficulty writing this in Typescript – when you import ./pure into index you get the following issues:

So, I'm wondering if anyone had a suggestion? I already can guess it'll be to do with the require call. Here's the code for pure.

type RendererArray = Array<{ required: string; renderer: string }>

const RENDERERS: RendererArray = [{ required: 'react-test-renderer', renderer: './native/pure' }]

function getRenderer (renderers: RendererArray): string {
  const hasDependency = (name: string) => {
    try {
      require(name)
      return true
    } catch {
      return false
    }
  }

  const [validRenderer] = renderers.filter(({ required }) => hasDependency(required))

  if (validRenderer) {
    return validRenderer.renderer
  } else {
    const options = renderers.map(({ renderer }) => `  - ${renderer}`).join('\n')
    throw new Error(`Could not auto-detect a React renderer.  Options are:\n${options}`)
  }
}

export default require(getRenderer(RENDERERS))

and for total transparency here's the code for index.ts

import { renderHook, act, cleanup } from './pure'

cleanup.autoRegister()

export { renderHook, act, cleanup }

[mpeyper]

@joshuaellis I think you are correct that the require call is unable to determine the type. I think we'll end up needing something like:

const { renderHook, act, cleanup } = require(getRenderer(RENDERERS)) as Renderer // this type does not exists AFAIK
export { renderHook, act, cleanup }

[joshuaellis]

Cheers @mpeyper i'll look into this a bit more with that direciton, I did think that would be the way to go. I've noticed we have some @types in depencies, just wondering if they should be there or devDependencies. I'm not sure personally but I wanted to raise it, I can also ensure when I add react-dom I put that @types into the correct place.

[mpeyper]

dependencies is correct for these ones as our public API uses their types so our TS consumers need them too.

It's possible that after your changes react-dom and react-test-renderer would become devDependencies as we won't be able to rely on either's types in our public API.

[joshuaellis]

This was released in v4.0.0