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.

Sign up for GitHub

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

Jump to bottom

fix: don't /// <reference types="jest" /> #9875

Merged
merged 4 commits into from Apr 24, 2020
Merged

fix: don't /// <reference types="jest" /> #9875

merged 4 commits into from Apr 24, 2020

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Apr 23, 2020

Summary

DefinitelyTyped/DefinitelyTyped#44033 (comment)

@jest/environment references types="jest" without a dependency on @types/jest:

$ npm install @jest/environment
$ > index.ts
$ tsc --types @jest/environment index.ts
node_modules/@jest/environment/build/index.d.ts:8:23 - error TS2688: Cannot find type definition file for 'jest'.

8 /// <reference types="jest" />
                        ~~~~

I'm assuming that the /// <reference types="jest" /> is unintended? but I haven't gotten to the bottom of why it gets added/whether it's a bug in the compiler?

By default, all visible @types packages are included in your compilation, but there are plenty of other visible @types packages that aren't added to the generated declaration. Presumably tsc only adds those that are actually used, but @jest/environment doesn't use @types/jest? at least it compiles fine with "types": ["node"].

Perhaps it's a symptom of microsoft/TypeScript#35343? I haven't dug any deeper. Meanwhile this workaround is cribbed from https://github.com/facebook/jest/blob/6cbd3cbbb4a6d261b65f8eec034146e4be426ff2/packages/jest-globals/tsconfig.json#L4-L5

Test plan

I ran yarn build and confirmed that after this change, the generated packages/jest-environment/build/index.d.ts no longer contains /// <reference types="jest" />

@facebook-github-bot
Copy link
Contributor

Hi @jablko!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@jablko jablko mentioned this pull request Apr 23, 2020
Closed
9 tasks
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@SimenB
Copy link
Member

SimenB commented Apr 24, 2020

Thanks! I'll see if I can add some test that verifies no d.ts file references jest then merge this 👍

@SimenB
Copy link
Member

SimenB commented Apr 24, 2020 •
edited

Pushed a commit that will make the TS build fail if there are any /// reference type that do not reference @types/node.

Looks like this on master

image

And passes on this branch

@SimenB SimenB merged commit c024dec into jestjs:master Apr 24, 2020
@SimenB
Copy link
Member

SimenB commented Apr 24, 2020

Thanks @jablko!

@jablko
Copy link
Contributor Author

jablko commented Apr 24, 2020

Awesome, thank you!

@douglascayers
Copy link

Thanks @SimenB @jablko

@douglascayers douglascayers mentioned this pull request Apr 25, 2020
Closed
ghostd pushed a commit to ghostd/jest that referenced this pull request Apr 26, 2020
@jablko @ghostd
fix: don't /// <reference types="jest" /> (jestjs#9875)
c2b0118
jeysal added a commit to SimenB/jest that referenced this pull request Apr 26, 2020
@jeysal
Merge remote-tracking branch 'upstream/master' into hoist-global-import
76f0c38 
* upstream/master:
  Prints the Symbol name into the error message with a custom asymmetric matcher (jestjs#9888)
  Show coverage of sources related to tests in changed files (jestjs#9769)
  fix: don't /// <reference types="jest" /> (jestjs#9875)
jeysal added a commit to mmkal/jest that referenced this pull request Apr 26, 2020
@jeysal
Merge remote-tracking branch 'upstream/master' into uglier-inline-sna…
78edd86 
…pshots

* upstream/master: (39 commits)
  Prints the Symbol name into the error message with a custom asymmetric matcher (jestjs#9888)
  Show coverage of sources related to tests in changed files (jestjs#9769)
  fix: don't /// <reference types="jest" /> (jestjs#9875)
  noCodeFrame respects noStackTrace (jestjs#9866)
  chore: update example to react-native@0.62 (jestjs#9746)
  Improve source map handling when instrumenting transformed code (jestjs#9811)
  Update .vscode/launch.json settings (jestjs#9868)
  chore: verify all packages have the same engine requirement (jestjs#9871)
  fix: pass custom cached realpath function to `resolve` (jestjs#9873)
  chore: mock stealthy-require in tests (jestjs#9855)
  chore: update resolve (jestjs#9872)
  chore: run CircleCI on node 14 (jestjs#9870)
  Add an option to vscode settings to always use workspace TS (jestjs#9869)
  fix(esm): handle parallel imports (jestjs#9858)
  chore: run CI on Node 14 (jestjs#9861)
  feat: add `@jest/globals` package for importing globals explici… (jestjs#9849)
  chore: bump resolve package (jestjs#9859)
  chore(runtime): simplify `createJestObjectFor` (jestjs#9857)
  chore: fix symlink creation failures on windows in tests (jestjs#9852)
  chore: skip mercurial tests when no hg installed (jestjs#9840)
  ...
jeysal added a commit to mmkal/jest that referenced this pull request Apr 26, 2020
@jeysal
Merge remote-tracking branch 'upstream/master' into uglier-inline-sna…
27cf6e0 
…pshots

* upstream/master: (39 commits)
  Prints the Symbol name into the error message with a custom asymmetric matcher (jestjs#9888)
  Show coverage of sources related to tests in changed files (jestjs#9769)
  fix: don't /// <reference types="jest" /> (jestjs#9875)
  noCodeFrame respects noStackTrace (jestjs#9866)
  chore: update example to react-native@0.62 (jestjs#9746)
  Improve source map handling when instrumenting transformed code (jestjs#9811)
  Update .vscode/launch.json settings (jestjs#9868)
  chore: verify all packages have the same engine requirement (jestjs#9871)
  fix: pass custom cached realpath function to `resolve` (jestjs#9873)
  chore: mock stealthy-require in tests (jestjs#9855)
  chore: update resolve (jestjs#9872)
  chore: run CircleCI on node 14 (jestjs#9870)
  Add an option to vscode settings to always use workspace TS (jestjs#9869)
  fix(esm): handle parallel imports (jestjs#9858)
  chore: run CI on Node 14 (jestjs#9861)
  feat: add `@jest/globals` package for importing globals explici… (jestjs#9849)
  chore: bump resolve package (jestjs#9859)
  chore(runtime): simplify `createJestObjectFor` (jestjs#9857)
  chore: fix symlink creation failures on windows in tests (jestjs#9852)
  chore: skip mercurial tests when no hg installed (jestjs#9840)
  ...
@SimenB
Copy link
Member

SimenB commented Apr 29, 2020

@jablko out it 25.5

@dependabot dependabot bot mentioned this pull request Mar 17, 2021
Merged
@github-actions
Copy link

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 May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jablko @facebook-github-bot @SimenB @douglascayers