Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

[several packages] Update jest matcher types #1239

Merged
merged 4 commits into from
Apr 17, 2020
Merged

Conversation

jgodson
Copy link
Contributor

@jgodson jgodson commented Jan 19, 2020

Description

This PR updates the jest matcher types to be compatible with types in jest 25.

This was updated from the original because it doesn't reflect the merged state.

Original Description:
When working on https://github.com/Shopify/web/pull/22447, I ran into issues when using toHaveReactProps. For some reason it was no longer inferring the typescript types of the React Components. I believe it was due to this change and due to the namespace no longer implementing the same interface it cause these issues.

This PR updates the @types/jest package and the Matcher interface in all packages to match.

Type of change

I'm not sure what type of change this should be. It's a breaking change for Typescript projects, but the update to the types package is the real reason, not these packages themselves

  • ast-utilities Minor: jest.Matchers type updated to match @types/jest version 25
  • graphql-testing Minor: jest.Matchers type updated to match @types/jest version 25
  • react-i18n Minor: jest.Matchers type updated to match @types/jest version 25
  • react-testing Minor: jest.Matchers type updated to match @types/jest version 25

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above

@GoodForOneFare
Copy link
Member

@lemonmade this change makes sense to me. Could you help with Jason's question about what type of version bump this should be in the packages that it touches, please?

Copy link
Member

@lemonmade lemonmade left a comment

Choose a reason for hiding this comment

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

Sounds good, unfortunately no way to make this a non-breaking change. Since this came as a minor version change on the Jest types, I think I'd do a minor release here and clearly document what Jest type versions coordinate to what package versions in the relevant changelogs.

@vsumner vsumner force-pushed the update-jest-matcher-types branch from 594d0ba to e32e761 Compare April 16, 2020 21:43
@@ -7,7 +7,7 @@ import {
declare global {
// eslint-disable-next-line @typescript-eslint/no-namespace
namespace jest {
interface Matchers<R> {
interface Matchers<R, T = {}> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vsumner vsumner force-pushed the update-jest-matcher-types branch from 80fb3d4 to a094839 Compare April 17, 2020 14:33
@jgodson jgodson changed the title [WIP] [several packages] Update jest matcher types [several packages] Update jest matcher types Apr 17, 2020
Copy link
Contributor

@marutypes marutypes left a comment

Choose a reason for hiding this comment

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

❤️ thanks for looking at this

@jgodson jgodson merged commit 16a6f04 into master Apr 17, 2020
@jgodson jgodson deleted the update-jest-matcher-types branch April 17, 2020 18:41
@alexandcote
Copy link
Contributor

@jgodson, can you check if you have the latest version of @types/jest

I think I already solve this in this PR.

#1295

@jgodson
Copy link
Contributor Author

jgodson commented Apr 18, 2020

Yep your PR fixed React testing, which this originally included as well. This updates the other packages. (I created this PR in January, but took forever to merge it 😅) . Also Victor just commited new changes for another type change in Jest 25 (that's why react-testing is also updated here).

I did forget to update the description though, which I've done now (sorry)

@alexandcote
Copy link
Contributor

My point is Matchers<R> should be valid because T is optional. I think we should only add T if we need it like here.

@jgodson
Copy link
Contributor Author

jgodson commented Apr 19, 2020

Good point, it wasn't optional when I first put the PR up, but it appears that's the changes in jest 25 that Victor added. It shouldn't cause any issues though either in that case? Maybe @vsumner can speak more to those changes as he was the one having issues this time.

@vsumner vsumner temporarily deployed to production April 20, 2020 17:01 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants