chore: drop old @types/jest^27 #3513

noomorph · 2022-05-16T09:54:44Z

Summary

I see that Jest 28 brings its own types, so probably there's no sense to maintain twice the same typings in your example apps.

Test plan

Actually, for some reason, I see failures. Could you explain why?

Does this PR introduce a breaking change?

Yes, but it should not 😢
No

Other information

coveralls · 2022-05-16T10:44:44Z

Pull Request Test Coverage Report for Build 2448505546

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 94.792%

Totals
Change from base Build 2448383619:	0%
Covered Lines:	1120
Relevant Lines:	1147

💛 - Coveralls

noomorph · 2022-05-16T11:59:59Z

Interesting. It doesn't reproduce in CI flow.

However, if you checkout this branch and:

git status # should show zero staged or unstaged changes
git clean -xdf # let's make sure there are no untracked files in the repo
cd examples/ts-only
npm install
npm test

you'll probably see the error mentioned above.

I also deleted package-lock.json and patched manually the json to:

   "devDependencies": {
-    "jest": "^28.0.2",
-    "ts-jest": "28.0.0-next.2",
+    "jest": "^28.0.0",
+    "ts-jest": "^28.0.0",
     "typescript": "~4.5.2"
   }
 }

to make sure the latest 28.x versions are installed, but unfortunately I had the same luck.

noomorph · 2022-05-16T12:02:42Z

Ah, I know why. That's because @types/jest is brought from the root package.json.

ahnpnl · 2022-05-16T12:15:55Z

actually you should remove this line

ts-jest/package.json

Line 74 in 7a7aba9

"@types/jest": {

it is not a breaking change since we removed mocked in v28

The dev dependencies have no impact on published package.

The example apps need to be updated though, probably switch all to @jest/globals.

noomorph · 2022-05-16T16:10:23Z

So, I removed "@types/jest" now from everywhere — I hope it does not conflict the vision of the project.

ahnpnl · 2022-05-16T16:37:50Z

We indeed need to use @jest/globals anywhere we are using jest, CI complained.

noomorph · 2022-05-16T16:41:59Z

Oh, now we're at the heart of the issue. 😄

I'm afraid @jest/globals does not effectively declare _{...it rather... exports??} any globals, compare DefinitelyTyped and @jest/globals:

DefinitelyTyped

@jest/globals

declare var beforeAll: jest.Lifecycle;
declare var beforeEach: jest.Lifecycle;
declare var afterAll: jest.Lifecycle;
declare var afterEach: jest.Lifecycle;
declare var describe: jest.Describe;
// ...

export declare const jest: Jest;
export declare const expect: JestExpect;
export declare const it: Global.GlobalAdditions['it'];
export declare const test: Global.GlobalAdditions['test'];
// ...

I'm not proficient enough in this part of TypeScript syntax, but I suspect declare and export declare behave differently.

As a weak confirmation, I force-pushed now a change to tsconfig.json, but it does not seem to help:

diff --git a/tsconfig.json b/tsconfig.json
index e9b685be..42d01c29 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -17,6 +17,6 @@
     "target": "es2015",
     "module": "ESNext",
     "lib": ["esnext"],
-    "types": ["node", "jest"]
+    "types": ["node", "@jest/globals"]
   }
 }

noomorph · 2022-05-16T16:52:34Z

I think I might need your help to advance further with this. I'm not sure if this is an issue with Jest 28 or with TS-Jest itself. 😢

ahnpnl · 2022-05-16T17:01:44Z

I think you just need to find any places where we are using jest. and add the import from @jest/globals that should work.

It is the issue caused by removing the type package from dependencies. If we simply add import jest from @jest/globals in each file we use jest. it will solve the issue.

noomorph · 2022-05-16T17:27:01Z

@ahnpnl this is exactly what I am putting under the doubt. See my repo which reproduces the issue with Jest and TypeScript only: https://github.com/noomorph/tsc-jest-28-issue

P. S. Just to be on the same page, I am at zero level with your project and its history and peculiarities. I can't triage whether it is miscoordination between your team and Jest team, or it is someone's specific bug, or maybe it is an intended behavior, so I need a perspective from someone knowledgeable. But definitely it is an interesting problem that probably has to be addressed eventually by someone. It just seemed so weird that I must use @types/jest@27 with jest@28 (according to the peerDependencies at least), 🤔 that's why I started this investigation at all.

ahnpnl · 2022-05-16T17:33:00Z

tsconfig.json

@@ -17,6 +17,6 @@
    "target": "es2015",
    "module": "ESNext",
    "lib": ["esnext"],
-    "types": ["node", "jest"]
+    "types": ["node", "@jest/globals"]


This one is the wrong one. This won’t work. You need to remove @jest/globals here and then open each test file which we are doing jest. and add an import statement.

import { jest } from ‘@jest/globals’;

An example is

ts-jest/e2e/ast-transformers/hoist-jest/__tests__/import-jest.test.js

Line 1 in 7a7aba9

import { jest } from '@jest/globals'

Previously when we have typing package @types/jest, typescript compiler understands jest as namespace. Now we don’t have that namespace anymore so it throws errors and the package @jest/globals doesn’t expose namespace

@ahnpnl sounds like a catastrophic change for anyone who's using Jest (i.e. one has to augment every single test file with an explicit import ...), doesn't it?

See the repo I mentioned: https://github.com/noomorph/tsc-jest-28-issue

The problem I'm seeing is that describe, beforeEach are not seen as globals for some reason.

If you mean specific tests which use jest. — okay, I'll do it. But I thought that the problem is a bit deeper than just a single variable.

Another way is something like
declare var jest: typeof import(‘@jest/globals’).jest; in a d.ts file, I hope it works the same as import everywhere. The same applies to describe and others.

Also make sure tsconfig picks that d.ts

What about the other global vars?

I think the best way to reduce the imports per test file is declaring a namespace globally, or declaring several global vars. It’s more like replicating what @types/jest does.

@ahnpnl yeah, sort of. I'll make a draft for a typings file and submit it for a review.

But still I feel like it is not morally correct to fix it only in the scope of a single repo — partially because it doesn't sit well in my head how @types/jest is going to live from now on... 🤔 Will they have 28th version doing exactly what you suggested or not. Won't it create a conflict with jest => node_modules/jest/package.json -> types and jest => node_modules/@types/jest -> ... ?

If they won't do it, then still it can't be that every project using Jest and TS has to redeclare these globals independently.

or even better, @jest/globals should change to something like vitest/globals https://vitest.dev/config/#globals

See their package.json entry https://github.com/vitest-dev/vitest/blob/03a54e0651f51b739b7833817d14d2cbcf474ede/packages/vitest/package.json#L32 Jest can do something like this, which helps a lot to migrate. This is exactly what we discuss here about declaring global var https://github.com/vitest-dev/vitest/blob/03a54e0651f51b739b7833817d14d2cbcf474ede/packages/vitest/globals.d.ts#L1

This discussion is long, but if you want globals typed you need to use @types/jest. I hope to land DefinitelyTyped/DefinitelyTyped#44365 at some point, but that hasn't happened yet. @jest/globals is not meant to be used in types in tsconfig.json, it's made to be explicitly imported

@SimenB thanks for shedding light on the DT situation. So it doesn't go away, ok.

noomorph · 2022-05-17T14:38:13Z

Today I have some progress, but the tests are failing for E2Es still. Need to look there separately.

src/legacy/ts-jest-transformer.spec.ts

src/legacy/config/config-set.spec.ts

mrazauskas · 2022-05-17T16:24:06Z

src/legacy/config/config-set.spec.ts

+  let readConfig!: SpyInstance<(...args: any[]) => { config?: any; error?: ts.Diagnostic }>
+  let parseConfig!: SpyInstance<(...args: any[]) => ts.ParsedCommandLine>


Suggested change

let readConfig!: SpyInstance<(...args: any[]) => { config?: any; error?: ts.Diagnostic }>

let parseConfig!: SpyInstance<(...args: any[]) => ts.ParsedCommandLine>

let readConfig!: SpyInstance<typeof ts.readConfigFile>

let parseConfig!: SpyInstance<typeof ts.parseJsonConfigFileContent>

Seems like jest.Mocked would do better. Working on it (jestjs/jest#12727).

Interestingly jest.mocked() can’t be used as replacement. I was in doubt if both of them are needed. Thanks for motivation. I will push forward with jest.Mocked.

src/legacy/config/config-set.spec.ts

noomorph · 2022-05-24T13:59:00Z

Hopefully, I fixed all the current present review notes, but let's run on CI again and see how it's doing.

ahnpnl · 2022-05-27T12:13:50Z

Would you pls rebase? I think all good to go

noomorph · 2022-05-29T16:00:54Z

Rebased.
P. S. Not sure if it is good to go – some e2e tests fail... 🤔 maybe we need to change something in their configs... 🤷‍♂️

noomorph · 2022-05-30T14:43:07Z

Hm, interesting. E2Es just hanged up. I'll try to look out for what went wrong but I might need some help maybe. Meanwhile, I'll fix lint issue and maybe try to debug 1-2 e2e tests.

guoyunhe · 2022-06-06T06:13:18Z

Great PR! How could this move forward? npm9 is throwing errors because of mismatch peerDependencies...

ahnpnl · 2022-06-06T09:49:03Z

Currently this PR only impacts internally. In release 28.0.4 @types/jest is no longer a peer dependency.

ahnpnl

I've included the custom global types to e2e which makes type check happy. We can merge this.

noomorph · 2022-06-06T15:19:19Z

@ahnpnl thanks a lot for looking into this! Although, it makes me a bit uncomfortable that there are multiple repetitive files now in examples like examples/js-with-babel/globals.d.ts. I hope this is in line with your vision.

ahnpnl · 2022-06-06T15:22:18Z

For now we can keep those duplicated files. Once Jest provides the global typings, those files can be removed later.

noomorph requested a review from kulshekhar as a code owner May 16, 2022 09:54

noomorph mentioned this pull request May 16, 2022

chore: upgrade internally to jest@28 wix/Detox#3395

Merged

ahnpnl reviewed May 16, 2022

View reviewed changes

ahnpnl mentioned this pull request May 16, 2022

[Bug]: breaking behavior for TypeScript globals compared to the DefinitelyTyped counterpart jestjs/jest#12853

Closed

mrazauskas reviewed May 17, 2022

View reviewed changes

src/legacy/ts-jest-transformer.spec.ts Outdated Show resolved Hide resolved

mrazauskas reviewed May 17, 2022

View reviewed changes

src/legacy/config/config-set.spec.ts Outdated Show resolved Hide resolved

mrazauskas reviewed May 17, 2022

View reviewed changes

src/legacy/config/config-set.spec.ts Show resolved Hide resolved

noomorph and others added 2 commits June 6, 2022 16:36

build: drop @types/jest^27 dependency

2ed4c53

test(e2e): include globals.d.ts to e2e and example apps

152b615

ahnpnl approved these changes Jun 6, 2022

View reviewed changes

ahnpnl merged commit 1342a4a into kulshekhar:main Jun 6, 2022

noomorph deleted the chore/remove-definitely-typed branch June 7, 2022 08:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: drop old @types/jest^27 #3513

chore: drop old @types/jest^27 #3513

noomorph commented May 16, 2022

coveralls commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

ahnpnl commented May 16, 2022 •

edited

noomorph commented May 16, 2022

ahnpnl commented May 16, 2022

noomorph commented May 16, 2022 •

edited

noomorph commented May 16, 2022

ahnpnl commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

ahnpnl May 16, 2022

ahnpnl May 16, 2022

noomorph May 16, 2022 •

edited

ahnpnl May 16, 2022 •

edited

noomorph May 16, 2022

ahnpnl May 16, 2022

noomorph May 16, 2022 •

edited

ahnpnl May 16, 2022 •

edited

SimenB May 16, 2022

noomorph May 16, 2022

noomorph commented May 17, 2022

mrazauskas May 17, 2022

noomorph commented May 24, 2022

ahnpnl commented May 27, 2022

noomorph commented May 29, 2022 •

edited

noomorph commented May 30, 2022 •

edited

guoyunhe commented Jun 6, 2022

ahnpnl commented Jun 6, 2022

ahnpnl left a comment

noomorph commented Jun 6, 2022

ahnpnl commented Jun 6, 2022

		let readConfig!: SpyInstance<(...args: any[]) => { config?: any; error?: ts.Diagnostic }>
		let parseConfig!: SpyInstance<(...args: any[]) => ts.ParsedCommandLine>

chore: drop old @types/jest^27 #3513

chore: drop old @types/jest^27 #3513

Conversation

noomorph commented May 16, 2022

Summary

Test plan

Does this PR introduce a breaking change?

Other information

coveralls commented May 16, 2022 • edited

Pull Request Test Coverage Report for Build 2448505546

💛 - Coveralls

noomorph commented May 16, 2022 • edited

noomorph commented May 16, 2022 • edited

ahnpnl commented May 16, 2022 • edited

noomorph commented May 16, 2022

ahnpnl commented May 16, 2022

noomorph commented May 16, 2022 • edited

noomorph commented May 16, 2022

ahnpnl commented May 16, 2022 • edited

noomorph commented May 16, 2022 • edited

ahnpnl May 16, 2022

Choose a reason for hiding this comment

ahnpnl May 16, 2022

Choose a reason for hiding this comment

noomorph May 16, 2022 • edited

Choose a reason for hiding this comment

ahnpnl May 16, 2022 • edited

Choose a reason for hiding this comment

noomorph May 16, 2022

Choose a reason for hiding this comment

ahnpnl May 16, 2022

Choose a reason for hiding this comment

noomorph May 16, 2022 • edited

Choose a reason for hiding this comment

ahnpnl May 16, 2022 • edited

Choose a reason for hiding this comment

SimenB May 16, 2022

Choose a reason for hiding this comment

noomorph May 16, 2022

Choose a reason for hiding this comment

noomorph commented May 17, 2022

mrazauskas May 17, 2022

Choose a reason for hiding this comment

noomorph commented May 24, 2022

ahnpnl commented May 27, 2022

noomorph commented May 29, 2022 • edited

noomorph commented May 30, 2022 • edited

guoyunhe commented Jun 6, 2022

ahnpnl commented Jun 6, 2022

ahnpnl left a comment

Choose a reason for hiding this comment

noomorph commented Jun 6, 2022

ahnpnl commented Jun 6, 2022

coveralls commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

ahnpnl commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

ahnpnl commented May 16, 2022 •

edited

noomorph commented May 16, 2022 •

edited

noomorph May 16, 2022 •

edited

ahnpnl May 16, 2022 •

edited

noomorph May 16, 2022 •

edited

ahnpnl May 16, 2022 •

edited

noomorph commented May 29, 2022 •

edited

noomorph commented May 30, 2022 •

edited