-
Notifications
You must be signed in to change notification settings - Fork 381
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
Remove TypeScript warning from README #251
Conversation
Codecov Report
@@ Coverage Diff @@
## master #251 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 315 315
Branches 72 72
=========================================
Hits 315 315 Continue to review full report at Codecov.
|
I'm not sure that this is the case. When you import some matchers manually and not others, you need to extend Or am I missing something here? |
Can you give a concrete example where typing is not properly working ATM? |
Ok, yes, you are right. Now I realized we have a different problem. If someone uses the custom matchers in this way, then they now have type definitions for all matchers, even those they did not import. They get auto-complete in jest-dom even. But then of course tests fail if they end up using the ones they did not import. I wonder if we should remove this section altogether (the reference to importing custom matchers and extending Would like to hear others' thoughts about this. |
@gnapse any news on this? |
Oh yes, sorry. Would you mind removing the entire section altogether? From where we start saying "Alternatively you can import only the matchers you need" (something along those lines). It's line 121 I think. If not don't worry, we can also merge this as is. Let me know. |
Typings is not properly working at the moment on my side with this example: import {
toBeInTheDocument,
toHaveClass
} from "@testing-library/jest-dom/matchers";
expect.extend({ toBeInTheDocument, toHaveClass });
https://codesandbox.io/s/cocky-microservice-qwjx5?file=/src/setupTests.ts |
The thing is we really do not support that and TypeScript at the same time. Not sure we can. My take on this was that we should remove this section altogether from the README. That's what I last requested from @MoSattler. Sadly I can't add changes to this PR, but I can start a new one superseding this one. |
@gnapse sorry for the delay. Did as you requested! |
@all-contributors add @MoSattler for docs |
I've put up a pull request to add @MoSattler! 🎉 |
🎉 This PR is included in version 5.11.5 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This does not see to be the case anymore, TS works fine since #11