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

Update Matchers interface #270

Merged
merged 1 commit into from Nov 12, 2019
Merged

Update Matchers interface #270

merged 1 commit into from Nov 12, 2019

Conversation

tobilen
Copy link

@tobilen tobilen commented Oct 28, 2019

the Matchers type of definitelytyped's jest package have been updated (see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/jest/index.d.ts#L639 ), which causes a compilation error since the interfaces don't match anymore.

See: enzymejs/enzyme-matchers#318 or testing-library/jest-dom#152

This PR changes the type to match the new one.

Copy link

@blainekasten blainekasten left a comment

Choose a reason for hiding this comment

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

Ahh this makes sense. What is this T type and what it is it used for?

@tobilen
Copy link
Author

tobilen commented Oct 29, 2019

@blainekasten as far as i can tell, for custom matchers passed to toMatchSnapshot assertions (see https://jestjs.io/docs/en/snapshot-testing#property-matchers ). not a 100% on that though

based on these lines: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/jest/index.d.ts#L838
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/jest/index.d.ts#L849

@tobilen
Copy link
Author

tobilen commented Nov 6, 2019

bump @probablyup can we get this through the door?

@quantizor quantizor merged commit ee8a43a into styled-components:legacy-v6 Nov 12, 2019
@quantizor
Copy link
Contributor

@tobilen does this need to be done for master as well?

@tobilen
Copy link
Author

tobilen commented Nov 12, 2019

most likely. see #269

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants