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: improve tsd runner setup #12199

Closed
wants to merge 2 commits into from
Closed

chore: improve tsd runner setup #12199

wants to merge 2 commits into from

Conversation

mrazauskas
Copy link
Contributor

@mrazauskas mrazauskas commented Dec 30, 2021

Summary

Should unblock #12198

Looks like tsd is trying to make sure that index.d.ts file exist in the rootDir or that it can be resolved looking at main or types fields of packages.json. The resolved file is not anyhow consumed later. https://github.com/SamVerschueren/tsd/blob/0fb92456b929e71bf6266101cb11d6cc90a241a5/source/lib/index.ts#L16-L30

Currently the runner is simply passing rootDir to tsd. This is tricky with mono-repo, because .d.ts files cannot be resolve from the root of the repo.

Might be it is possible to improve the runner to be smarter with mono-repos. Perhaps at the moment it is enough to set rootDir explicitly for each package which has tsd tests.

Test plan

All tests must pass.

@codecov-commenter
Copy link

codecov-commenter commented Dec 30, 2021

Codecov Report

Merging #12199 (dcd8bde) into main (a427bfc) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #12199   +/-   ##
=======================================
  Coverage   67.71%   67.71%           
=======================================
  Files         328      328           
  Lines       16993    16993           
  Branches     4818     4818           
=======================================
  Hits        11507    11507           
  Misses       5453     5453           
  Partials       33       33           
Impacted Files Coverage Δ
packages/jest-types/__typechecks__/config.test.ts 0.00% <ø> (ø)
packages/jest-types/__typechecks__/expect.test.ts 0.00% <ø> (ø)
packages/jest-types/__typechecks__/globals.test.ts 0.00% <ø> (ø)
packages/jest-types/__typechecks__/jest.test.ts 0.00% <ø> (ø)

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 a427bfc...dcd8bde. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Dec 30, 2021

I don't think one project for each module is the correct solution. Our current setup works fine with 1.1.0, seems like a regression in the runner?

@mrazauskas
Copy link
Contributor Author

I don't think one project for each module is the correct solution.

It is not very good, I agree. Just push alternative solution. What you think?

I was trying to point out that tsd simply checks if .d.ts file exist. It does not check if it is valid .d.ts, if it is included in the program, or if it is a declaration file at all? One could point to batman.txt and that would work. Strange.

Seems like to pass this check the vanilla tsd should be installed in each package and should be run from package, not from root of mono-repo. Might be I missed something, but this is where the idea to use projects came from.

In any case the check does not seem to be very valuable and can be silenced in different ways. Currently @type ../ points tsd to the root of a package, it finds package.json with types field there and is happy.

Regarding the regression. If I remember it right, the runner now works as a drop in replacement for tsd. I was trying to follow tsd internals. Will double check it later, but there shouldn’t be any regression.

@mrazauskas
Copy link
Contributor Author

Here is the root of the problem. If @type pragma is not provided, v1.1.0 was still returning some path for typingsFile, but v1.2.0 returns undefined. I made this change in the runner. It seemed logic.

If typingsFile is falsy, tsd is trying to find typings in the root dir.

Funny part. If typingsFile is defined, tsd simply expects it to be a path which exist in the file system. Any path.

Since v1.1.0 was always returning some path, the check passed. So.. might be I introduced a regression in the runner. At the same time, parsing of @type pragmas seemed to be buggy.

Another idea: to silence that check in the runner. I mean, the check done by tsd. Have to think about it.

@mrazauskas
Copy link
Contributor Author

Found better solution without workarounds, see jest-community/jest-runner-tsd#41

@mrazauskas mrazauskas closed this Jan 4, 2022
@mrazauskas mrazauskas deleted the fix-tsd-runner-setup branch January 4, 2022 09:26
@github-actions
Copy link

github-actions bot commented Feb 4, 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 Feb 4, 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.

4 participants