Components: Add support for TypeScript unit tests #39436
Conversation
ciampo
added
Framework
ciampo
force-pushed
the
feat/typescript-unit-tests
branch
from
b6dbe4e to
dd0c989
Compare
|
Hey @sirreal — sorry for the ping again. I opened this PR to introduce some of the changes discussed in #39341. But the fact that I chose to refactor to TypeScript another test ( Maybe it's related to the fact that the new project for tests only includes files under |
ciampo
force-pushed
the
feat/typescript-unit-tests
branch
from
d58fce7 to
9ffcfb1
Compare
|
Size Change: 0 B Total Size: 1.23 MB ℹ️ View Unchanged
|
|
Quick update:
I spent some time looking into these errors — it turns out that they are not caused by this PR, they were only surfaced by it. This can be observed by checking out
See screenshot
In both cases, I was finally able to solve the issue by using This should unlock further work on this PR (the CI checks should be able to build the project!). I will also look into addressing the feedback from this #39436 (comment) Nonetheless, how can we make sure that the types that we generate when building our packages are error-free? The error that was surfaced by this PR is currently on |
ciampo
force-pushed
the
feat/typescript-unit-tests
branch
from
f7be417 to
7482395
Compare
ciampo
force-pushed
the
feat/typescript-unit-tests
branch
from
7482395 to
99d2f9f
Compare
|
Update:
I believe that this PR is ready for a final round of review if we think that this approach is good enough to move forward. Here's a summary of the other approach that was discussed (click to explan)To recap, the main alternative approach that was discussed was to rewrite all of the jest config to TypeScript (as also suggested previously in #39341 (comment)). This wouldn't be a trivial task (for the best results, It would also allow us to rewrite the custom |
ciampo
requested review from
sirreal,
dmsnell,
mirka,
gziolo and
sarayourfriend
|
Thanks for engaging with my feedback. I don't feel like there's anything else I can contribute review-wise because I don't understand the implications fully of the changes to the main |
There was a problem hiding this comment.
I discovered a wrinkle while testing this out now. First, apply this patch:
diff --git a/tsconfig.test.json b/tsconfig.test.json
index 85ff605e1d..433b1274d4 100644
--- a/tsconfig.test.json
+++ b/tsconfig.test.json
@@ -1,6 +1,8 @@
{
"extends": "./tsconfig.base.json",
"compilerOptions": {
+ "allowJs": false,
+ "checkJs": false,
"noEmit": true,
"emitDeclarationOnly": false,
"types": [This seems straightforward. We only want to typecheck TypeScript test files so we change this from the settings in the extended config. Now let's compile the test project: node_modules/.bin/tsc --build tsconfig.test.json:
packages/components/src/text/test/index.tsx:10:24 - error TS7016: Could not find a declaration file for module '../../utils'. 'packages/components/src/utils/index.js' implicitly has an 'any' type.
10 import { COLORS } from '../../utils';
~~~~~~~~~~~~~
packages/components/src/text/test/index.tsx:11:22 - error TS7016: Could not find a declaration file for module '../'. 'packages/components/src/text/index.js' implicitly has an 'any' type.
11 import { Text } from '../';
~~~~~
Found 2 errors.
Oops 🙂
What's happening?
We expected to compile all the application code, that's why we added references to every single project. Then, the tests should discover and use the declarations that were generated.
What happened instead? The short version is that we're accessing files from tests like import { Text } from '../. No declaration file exists for the file, so tsc tries to build it. Even though we configured our project to only include files immediately under test directories, the compiler finds that import so it becomes part of the project and we try to build it.
As-is, this PR compiles all the projects in a first pass, then recompiles just a couple files (including application code) of the files that are part of the test dependency graph. That isn't what we intended at all!
I was hoping that this problem was limited to @wordpress/components because it's missing the types field in package.json that points to the build-types directory. Unfortunately that does not seem to be the case 😞 Because we're importing relative modules (../…) from inside the same package, TypeScript doesn't have a chance to resolve these files from the package's type declarations 😬
I'm not sure what the best way forward is right now, I need to think about it a bit and do some testing. I'd be happy to chat about the problem and discuss possible solutions.
|
Does it mean that, basically, a test written in TypeScript should have all of its local dependencies also written in TypeScript? Would adding |
No. The problem I'm highlighting is that:
We expect to import
No. It's possible it could make errors go away, but I think we'd just lose type information for those files (imports from those modules would become |
I think that makes sense at this point. The main drawback is that we pollute the application code with test types (jest globals) but in practice that doesn't usually cause problems. |
|
Alright, I'll proceed an implement this idea in the next days — of course feel free to propose other approaches if they come to mind! |
…ents' existing project
ciampo
force-pushed
the
feat/typescript-unit-tests
branch
from
e3fae6f to
e0c3506
Compare
|
Rebased, and pushed changes to move away from a dedicated |
ciampo
changed the title
What?
Following the exploratory work in #39341, this PR adds the necessary configuration in order to add unit tests written in TypeScript to the repo.
Why?
Having unit tests in TypeScript is a way to ensure that we type-check a component from the point of view of a consumer (and not just internally).
This PR will allow us to refactor to TypeScript unit tests in the
@wordpress/componentspackage, in the context of the wider package refactor (#35744).How?
tsconfig.test.json)toMatchStyleDiffSnapshot)eslintconfig to include TypeScript tests in the "development files" glob matcher<Text />component to make sure that the above changes work for TypeScript unit testsTesting Instructions
npm run build && npx tsc -p packages/components/tsconfig.jsondoesn't output any errors to console.packages/components/src/text/test/index.tsxin an IDE supporting TypeScript:npx tsc -p packages/components/tsconfig.json