UnitControl: refactor unit tests to TypeScript and modern RTL
#39341
Conversation
ciampo
added
[Type] Enhancement
| @@ -1,6 +1,7 @@ | |||
| /** | |||
| * External dependencies | |||
| */ | |||
| import { describe, expect, it } from '@jest/globals'; | |||
There was a problem hiding this comment.
When working in JS, these globals are already in scope. In TypeScript, I had to explicitly include them (which, honestly, I don't mind since it feels less magic)
There was a problem hiding this comment.
The globals are still in scope, TypeScript just doesn't know about them. We need to add jest to the types array so that its types are loaded. We never import jest so its types are never loaded (unless we put them in the types array:
But this tsconfig file currently represents this entire project, it would make these globals become global which isn't accurate.
The way I've structured this before is to make 2 projects (2 tsconfigs):
tsconfig.json: the application code, excludes tests files (something like**/tests/in this case?)tsconfig.tests.json- only includes tests files. This is where we include the jest and testing-library types. The test project includes a reference to (depends on) the main project.
packages/components/package.json
Outdated
| "devDependencies": { | ||
| "@jest/globals": "27.5.1", | ||
| "@testing-library/react": "11.2.2" | ||
| }, |
There was a problem hiding this comment.
It looks like I can't list devDependencies in a package's package.json, but at the same time I have to list them in this file if I want to import them explicitly (e.g. importing @jest/globals in the unit test file).
What's the best solution here? Moving them to the root package.json in the devDependencies and then listing them again in this file's dependencies?
There was a problem hiding this comment.
I don't have a definitive answer as I'm not familiar with how deps are set up now.
I can say we shouldn't list these as direct dependencies of this package, that would cause consumers to download them when they're absolutely not necessary to use this package.
|
Based on testing-library's own instructions (https://github.com/testing-library/jest-dom#with-typescript) we need to be able to include the This presents some problems for the current monorepo tsconfig situation as we rely on It also means everything we import in there needs to be TS enabled (mostly this relates to the You'll need to refactor all the existing tsconfig's to move the rootDir to the root of the repository otherwise TypeScript will complain that you're including a file in the configuration that is outside the rootDir for each individual tsconfig. You could only do this for the components package and include the I also tried seeing if it was possible to add the I think the best route is the following (to be done in a separate PR probably):
After that I think the matches would start working. In my brief experiment the types were at least getting included and some of the errors went away. I'll note that there are also some legitimate errors, missing |
|
I pushed a fix for the issues where TypeScript didn't have global types. Ideally, I think tests would be treated as a separate project to avoid polluting the main project with globals that don't exists, but it's not a big risk. I described the setup a bit here #39341 (comment) |
|
Size Change: 0 B Total Size: 1.16 MB ℹ️ View Unchanged
|
ciampo
force-pushed
the
refactor/unit-control-unit-tests-typescript
branch
from
db32012
to
15ff9dd
Compare
|
Thank you @sarayourfriend and @sirreal for the help so far! After a couple of @sirreal 's commits, it looks like we've got a direction: creating separate For some reasons, I had to add At the moment, running Although my VSCode still shows a TS error for the Since IDE support is important, I wonder if we should rework how we expose/extend our configs (I found this comment), or that we try the approach described by @sarayourfriend ? To recap, I think I managed to push a couple of commits that fix issues, but they seem hacky and I don't really know why they worked 😅 Any thoughts? |
|
I've reviewed the tsconfig changes and it seems like a reasonable setup. I think this is a good starting point 👍 One nice thing is that the setup for non-test code is practically untouched. That's been working pretty well and these changes shouldn't disrupt it. Certainly, there will be improvements to this setup but this seems like the place to start. One side note: I noticed the TypeScript version used in this project is a couple versions old. It would be nice to get that dependency updated. |
|
Closing this PR in favour of #39436, where I polished the approach taken in this PR and removed the changes to
I'll see if I can work on that in the next days. |
What?
This PR refactors
UnitControl's unit tests from JS to TypeScript, and partially rewrites some tests to use modern techniques (mostly using@testing-library's methods).This PR is a WIP. TODO:
package.json)input's haverole="spinbutton' and some haverole"textbox"Why?
This is part of the
@wordpress/components's TypeScript migration effort (#35744).Using TypeScript unit tests (instead of JS) is a great way to type-check our components from the point of view of their consumers, and thus catch early bugs and API inconsistencies.
How?
Testing Instructions
UnitControlworks as expected in Storybook and in the block editor