Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(type tests): bump jest-runner-tsd #12299

Merged
merged 5 commits into from
Feb 4, 2022
Merged

chore(type tests): bump jest-runner-tsd #12299

merged 5 commits into from
Feb 4, 2022

Conversation

mrazauskas
Copy link
Contributor

Summary

@SimenB As agreed, let’s put updated jest-runner-tsd to work.

Fixes #12198

Test plan

Green CI

package.json Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #12299 (0f2c010) into main (7509d25) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12299      +/-   ##
==========================================
- Coverage   67.31%   67.28%   -0.04%     
==========================================
  Files         328      328              
  Lines       17297    17305       +8     
  Branches     5065     5065              
==========================================
- Hits        11644    11643       -1     
- Misses       5620     5629       +9     
  Partials       33       33              
Impacted Files Coverage Δ
packages/jest-types/__typechecks__/config.test.ts 0.00% <ø> (ø)
packages/jest-types/__typechecks__/expect.test.ts 0.00% <0.00%> (ø)
packages/jest-types/__typechecks__/globals.test.ts 0.00% <ø> (ø)
packages/jest-types/__typechecks__/jest.test.ts 0.00% <ø> (ø)
packages/expect/src/utils.ts 96.09% <0.00%> (-0.49%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7509d25...0f2c010. Read the comment docs.

@mrazauskas
Copy link
Contributor Author

Brief motivation / thinking on the compiler configuration for type tests (in other words, the tsconfig.json files in __typechecks__ directories):

In this case the type tests run agains the build artefacts. Ideally the test file should be written from user perspective, or: how these types will be consumed by the user?

The root tsconfig.json or tsconfig.json files inside src folders are used to build the libraries. They could do the job for type testing too, but isn’t it more straight forward to add simple tsconfig.json to each __typechecks__ directory? This one is part of the user story. Straight forward.

Another good question: which compiler options are important for the type testing? What is being tested? In this case we are testing types of returned values, argument types, number of arguments, type of this (inside expect.extend), assignability (config test), generic type arguments (expect test). Did I miss something? Seems like "strict": true is obviously important. It would change a lot in most of the cases.

I think having explicit compiler configuration for type tests is good solution. It is explicit, predictable and flexible (does not depend on anything).

package.json Show resolved Hide resolved
@mrazauskas mrazauskas requested a review from SimenB February 4, 2022 13:48
@@ -0,0 +1,9 @@
{
"extends": "@tsconfig/node10/tsconfig.json",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we create a tsconfig.test.json in root which has all this stuff instead of repeating in each package?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Perhaps .tsd pre-suffix is good idea? Also for jest.config.tsd.js?

packages/expect/__typechecks__/tsconfig.json Outdated Show resolved Hide resolved
@Biki-das
Copy link
Contributor

Biki-das commented Feb 4, 2022

@mrazauskas wow!, I personally faces this issue, a while ago

@SimenB SimenB merged commit 8a62d98 into jestjs:main Feb 4, 2022
@SimenB
Copy link
Member

SimenB commented Feb 4, 2022

Thank you! This is awesome work 👍

@mrazauskas
Copy link
Contributor Author

@SimenB Thanks. I was typing motivation. Will leave it here:

  • only one tsconfig.json in the root of the repo;
  • the repo root tsconfig.json defines only compilerOptions;
  • package root tsconfig.jsons extend it and define their own includes, excludes, references, etc.;
  • inside type tests directories the repo root tsconfig.json is extended with few overrides;
  • (not implemented) test directories could also extend the repo root config, for example overriding "noUnusedLocals": false.

Currently all tests are excluded in the root tsconfig.json and this makes it hard to use the root config as a single source of truth. In the other hand, this proposal has lots of repetition in package root tsconfig.jsons. At the same time this repetition is very similar to the one in .npmignore files have.

@mrazauskas mrazauskas deleted the chore-bump-jest-runner-tsd branch February 4, 2022 18:45
@github-actions
Copy link

github-actions bot commented Mar 7, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants