testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.26k stars 466 forks source link

chore: Migrate `ByRole` to TypeScript #1186

Closed DaniAcu closed 1 year ago

DaniAcu commented 1 year ago

What: Migrate role.js to role.ts.

Why:

Because is the only file that is in js. I wold like to add more inteligence to ByRole, but for any other thing we need to add it first as ts file.

How:

Checklist:

codesandbox-ci[bot] commented 1 year ago

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 9e5d4b4b5634823abda07c912be5d2596d4dacb2:

Sandbox Source
react-testing-library-examples Configuration
DaniAcu commented 1 year ago

@eps1lon Ohh okay, thanks. I didn't know that but that make sense. Let me update the PR to just have ts migration here. Thanks, again

codecov[bot] commented 1 year ago

Codecov Report

Merging #1186 (d72f79a) into alpha (25dc8a9) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head d72f79a differs from pull request most recent head 9e5d4b4. Consider uploading reports for the commit 9e5d4b4 to get more accurate results

@@            Coverage Diff            @@
##             alpha     #1186   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          998      1000    +2     
  Branches       327       328    +1     
=========================================
+ Hits           998      1000    +2     
Flag Coverage Δ
node-12 100.00% <100.00%> (?)
node-14 100.00% <100.00%> (ø)
node-16 100.00% <100.00%> (ø)
node-18 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/query-helpers.ts 100.00% <ø> (ø)
src/queries/role.ts 100.00% <100.00%> (ø)
src/helpers.ts 100.00% <0.00%> (ø)
src/queries/label-text.ts 100.00% <0.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

DaniAcu commented 1 year ago

@eps1lon I delayed more in make the fixes becuase lint is not working locally. image

So, I used the github jobs to check and fix it.

Related to the changes, I think that the types are not completly clean or organized. I will try to get into it a little bit more now that roles are also in ts.

Thanks for awesome welcoming to contribute here, I'm happy to could do it.

DaniAcu commented 1 year ago

@eps1lon there anything more that i need to change? 🤔

DaniAcu commented 1 year ago

@eps1lon I updated based on main and fix a error after merge. Could you re check it?

DaniAcu commented 1 year ago

@eps1lon hi, i keep the changes simple and i dismiss any change that affect run time. There some ts comments, but I think it's fine for the migration. There others things that need to be change in general, but for the proposal of this PR, i think that with this changes is enough.

Let me know if there any thing that you look weird in this changes.

And thanks in advance for the time that you take to revisit it

eps1lon commented 1 year ago

@all-contributors add @DaniAcu for code

allcontributors[bot] commented 1 year ago

@eps1lon

I've put up a pull request to add @DaniAcu! :tada: