fix: update min ts version of jest & co to 3.8 to allow bumping dependencies
#48473
Conversation
|
I just found #43613 where we do the same bumping of TS 🤦😂 So I think this is definitely the way to go. |
G-Rath
force-pushed
the
jest-update-ts-min-and-dependencies
branch
from
b6a70f7
to
68b52e1
Compare
| @@ -3,7 +3,7 @@ | |||
| // Definitions by: Christopher E. Woodland <https://github.com/cwoodland> | |||
| // Johnny Li <https://github.com/johnny4753> | |||
| // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped | |||
| // TypeScript Version: 3.1 | |||
| // TypeScript Version: 3.8 | |||
|
|
|||
| /// <reference types='jest'/> | |||
There was a problem hiding this comment.
I don't actually think this is needed - I can't find any evidence of @types/jest being used in this file, but will look into it further once I've got CI green.
There was a problem hiding this comment.
It seems that the original reference was to jasmine, and was because it was argumenting the global jasmine namespace.
However, while v2 of @types/frisby removed this argumenting, the reference was kept in because the author wanted to have @types/jasmine pulled in because the library used jasmine?
I can understand why, and am not against it, but I think it would be better to remove this reference since frisby.js doesn't list jest as a dependency (not even as a peerDependency) nor does it require/import it anywhere, and because people using TypeScript will need to install @types/jest anyway as they'll be consuming them directly.
Additionally, the source code of frisby still references jasmine rather than jest, and was originally switched to jest as the jasmine types were causing problems (#32191).
@cwoodland @johnny4753 thoughts?
There was a problem hiding this comment.
I've made #48935 to remove the reference.
|
Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files. frisby (unpkg)was missing the following properties:
jest-image-snapshot (unpkg)was missing the following properties:
jest-json-schema (unpkg)was missing the following properties:
jest-when (unpkg)was missing the following properties:
|
typescript-bot
moved this from Needs Author Action
to Needs Maintainer Review
in New Pull Request Status Board
|
@G-Rath Thank you for submitting this PR! This is a live comment which I will keep updated. 15 packages in this PR
Code ReviewsThis PR can be merged once it's reviewed. Status
All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 48473,
"author": "G-Rath",
"owners": [
"JoshuaKGoldberg",
"tkrotoff",
"jfm710",
"cwoodland",
"johnny4753",
"octogonz",
"iclanton",
"erbridge",
"mike-d-davydov",
"saitonakamura",
"dawnmist",
"peterblazejewicz",
"geovanisouza92",
"deadNightTiger",
"lvl99",
"joemitchard",
"jonasheinrich",
"aldentaylor",
"immanuel192",
"gstamac",
"sehsyha",
"NoHomey",
"jwbay",
"asvetliakov",
"alexjoverm",
"epicallan",
"ikatyang",
"wsmd",
"JamieMason",
"douglasduteil",
"ahnpnl",
"joshuakgoldberg",
"UselessPickles",
"r3nya",
"hotell",
"sebald",
"andys8",
"antoinebrault",
"ExE-Boss",
"quassnoi",
"Belco90",
"tonyhallett",
"ycmjason",
"devanshj",
"pawfa",
"regevbr",
"gnapse",
"jgoz",
"smacpherson64",
"mistic100"
],
"dangerLevel": "MultiplePackagesEdited",
"headCommitAbbrOid": "68b52e1",
"headCommitOid": "68b52e14810df2587af9a101fa3f67b6f399b0ed",
"mergeIsRequested": true,
"stalenessInDays": 0,
"lastPushDate": "2020-10-04T01:23:12.000Z",
"reopenedDate": "2020-10-04T01:35:52.000Z",
"lastCommentDate": "2020-10-20T06:53:50.000Z",
"maintainerBlessed": true,
"reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/48473/files",
"hasMergeConflict": false,
"authorIsOwner": false,
"isFirstContribution": false,
"popularityLevel": "Critical",
"newPackages": [],
"packages": [
"expect-puppeteer",
"frisby",
"heft-jest",
"jest-axe",
"jest-expect-message",
"jest-image-snapshot",
"jest-in-case",
"jest-json-schema",
"jest-matcher-one-of",
"jest-plugin-context",
"jest-specific-snapshot",
"jest-when",
"jest",
"testing-library__jest-dom",
"wordpress__jest-console"
],
"files": [
{
"path": "types/expect-puppeteer/index.d.ts",
"kind": "definition",
"package": "expect-puppeteer"
},
{
"path": "types/frisby/index.d.ts",
"kind": "definition",
"package": "frisby"
},
{
"path": "types/heft-jest/index.d.ts",
"kind": "definition",
"package": "heft-jest"
},
{
"path": "types/jest-axe/index.d.ts",
"kind": "definition",
"package": "jest-axe"
},
{
"path": "types/jest-expect-message/index.d.ts",
"kind": "definition",
"package": "jest-expect-message"
},
{
"path": "types/jest-image-snapshot/index.d.ts",
"kind": "definition",
"package": "jest-image-snapshot"
},
{
"path": "types/jest-image-snapshot/v2/index.d.ts",
"kind": "definition",
"package": "jest-image-snapshot"
},
{
"path": "types/jest-in-case/index.d.ts",
"kind": "definition",
"package": "jest-in-case"
},
{
"path": "types/jest-json-schema/index.d.ts",
"kind": "definition",
"package": "jest-json-schema"
},
{
"path": "types/jest-matcher-one-of/index.d.ts",
"kind": "definition",
"package": "jest-matcher-one-of"
},
{
"path": "types/jest-plugin-context/index.d.ts",
"kind": "definition",
"package": "jest-plugin-context"
},
{
"path": "types/jest-specific-snapshot/index.d.ts",
"kind": "definition",
"package": "jest-specific-snapshot"
},
{
"path": "types/jest-when/index.d.ts",
"kind": "definition",
"package": "jest-when"
},
{
"path": "types/jest/index.d.ts",
"kind": "definition",
"package": "jest"
},
{
"path": "types/jest/package.json",
"kind": "package-meta-ok",
"package": "jest"
},
{
"path": "types/testing-library__jest-dom/index.d.ts",
"kind": "definition",
"package": "testing-library__jest-dom"
},
{
"path": "types/wordpress__jest-console/index.d.ts",
"kind": "definition",
"package": "wordpress__jest-console"
}
],
"hasDismissedReview": false,
"ciResult": "pass",
"lastReviewDate": "2020-10-04T22:26:49.000Z",
"firstApprovalDate": "2020-10-04T04:57:06.000Z",
"reviewersWithStaleReviews": [],
"approvalFlags": 2,
"isChangesRequested": false
} |
|
🔔 @JoshuaKGoldberg @tkrotoff @jfm710 @cwoodland @johnny4753 @octogonz @iclanton @erbridge @mike-d-davydov @saitonakamura @dawnmist @peterblazejewicz @geovanisouza92 @deadNightTiger @lvl99 @joemitchard @jonasheinrich @aldentaylor @immanuel192 @gstamac @sehsyha @NoHomey @jwbay @asvetliakov @alexjoverm @epicallan @ikatyang @wsmd @JamieMason @douglasduteil @ahnpnl @JoshuaKGoldberg @UselessPickles @r3nya @Hotell @sebald @andys8 @antoinebrault @ExE-Boss @quassnoi @Belco90 @tonyhallett @ycmjason @devanshj @pawfa @regevbr @gnapse @jgoz @smacpherson64 @mistic100 — please review this PR in the next few days. Be sure to explicitly select |
|
/cc @SimenB |
| @@ -3,7 +3,7 @@ | |||
| // Definitions by: Pete Gonzalez <https://github.com/octogonz> | |||
| // Ian Clanton-Thuon <https://github.com/iclanton> | |||
| // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped | |||
| // TypeScript Version: 3.5 | |||
| // TypeScript Version: 3.8 | |||
There was a problem hiding this comment.
Heft currently ships with Jest 25. Seems like this change should wait until Heft upgrades its Jest dependency.
There was a problem hiding this comment.
Any idea of when that might be?
I don't think this'll be a problem so long as Heft can support TypeScript 3.8, which by the looks of it it can.
|
👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings. Let’s review the numbers, shall we? expect-puppeteer/v*Comparison details for expect-puppeteer/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. frisby/v*Comparison details for frisby/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. heft-jest/v*Comparison details for heft-jest/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-axe/v*Comparison details for jest-axe/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-expect-message/v*Comparison details for jest-expect-message/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-image-snapshot/v*Comparison details for jest-image-snapshot/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-image-snapshot/v*Comparison details for jest-image-snapshot/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-in-case/v*Comparison details for jest-in-case/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-json-schema/v*Comparison details for jest-json-schema/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-matcher-one-of/v*Comparison details for jest-matcher-one-of/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-plugin-context/v*Comparison details for jest-plugin-context/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-specific-snapshot/v*Comparison details for jest-specific-snapshot/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest-when/v*Comparison details for jest-when/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. jest/v*Comparison details for jest/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. testing-library__jest-dom/v*Comparison details for testing-library__jest-dom/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. wordpress__jest-console/v*Comparison details for wordpress__jest-console/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
typescript-bot
added
the
Perf: Same
typescript-bot
added
the
Owner Approved
| @@ -28,6 +28,7 @@ | |||
| // Pawel Fajfer <https://github.com/pawfa> | |||
| // Regev Brody <https://github.com/regevbr> | |||
| // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped | |||
| // Minimum TypeScript Version: 3.8 | |||
There was a problem hiding this comment.
For the sake of consistency, this should be:
| // Minimum TypeScript Version: 3.8 | |
| // TypeScript Version: 3.8 |
There was a problem hiding this comment.
See, I'd prefer that for consistency, but the readme says it should have that word 🤔
|
So as a conceptual change, I get where this PR is going (and making it feasible so the module I think we need a @SimenB (or other Jest team member) sign-off for it because the TS minimum version change is gonna break a few builds ;) |
|
Sign-off on what? The change itself seems correct to me - it should have happened when Are you asking for some guarantee the TS version will remain stable in the future? If so I can say that Jest 26 will keep 3.8 as minimum (we verify compatibility on CI). No current plans to bump that minimum in the next major (although I really want labeled tuples, I don't think it's worth the downstream breakage). Regarding breakage of consumers of this package - maybe wait for Jest 27? That's still months away, so might not be worth it, dunno. Your call 👍 |
elibarzilay
moved this from Needs Maintainer Review
to Waiting for Code Reviews
in New Pull Request Status Board
typescript-bot
added
the
Self Merge
typescript-bot
moved this from Waiting for Code Reviews
to Waiting for Author to Merge
in New Pull Request Status Board
|
I agree with @SimenB re this should have been done for Jest 26, so if things break they break. |
|
Ready to merge |
typescript-bot
moved this from Waiting for Author to Merge
to Recently Merged
in New Pull Request Status Board
|
@orta @elibarzilay have I broken the bot, or is it just a slow day..? 🤔 (maybe some sort of status comment could be useful, saying "I've got this queued for publishing"?) |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
731 -> 696 packages 🎉 (since I'm no longer duplicating |
This is required in order to eventually switch to sourcing types from
jestdirectly (#44365), and also to allow us to update our dependency ranges for@types/jestto pull in the right version - right now we're pulling in v24 packages, which in addition to being the wrong type means dependency trees tend to have some huge duplication as@typescomes beforejest(so package managers tend to put the v24 packages at the top level).After this is merged, the minimal TS version supported by
@types/jest& packages that reference@types/jestwill need to be updated wheneverjestups their min TS version.While this could be pretty breaking for some, we can't really avoid it as TS 3.8 doesn't become the min for DT until 2022 by which time
jestmight have a min version of TS 5 or something, so we'll be back in the exact same spot.This pain should be once-off since afterwards we'll just be matching the min version as directed by
jest.Also:
See #45603, #47545, #48426, and maybe a few other PRs & issues.
Creating as a draft initially as I expect this to go somewhat horribly wrong for the first few CI runs 😬