[@testing-library/jest-dom] Fix matchers type making the global expect unsafe#66247
[@testing-library/jest-dom] Fix matchers type making the global expect unsafe#66247kryops wants to merge 1 commit intoDefinitelyTyped:masterfrom
Conversation
|
@kryops Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PR
Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 66247,
"author": "kryops",
"headCommitOid": "c941395183fd9d1f0f1d893158f1eff732dbf430",
"mergeBaseOid": "6be113476bef803465cd7e1a96a14a96154351f3",
"lastPushDate": "2023-08-01T12:41:22.000Z",
"lastActivityDate": "2023-08-15T14:21:35.000Z",
"hasMergeConflict": true,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "testing-library__jest-dom",
"kind": "edit",
"files": [
{
"path": "types/testing-library__jest-dom/matchers.d.ts",
"kind": "definition"
},
{
"path": "types/testing-library__jest-dom/test/testing-library__jest-dom-extend-matchers-tests.ts",
"kind": "test"
},
{
"path": "types/testing-library__jest-dom/test/testing-library__jest-dom-global-tests.ts",
"kind": "test"
}
],
"owners": [
"gnapse",
"jgoz",
"smacpherson64",
"AndrewLeedham"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "AndrewLeedham",
"date": "2023-08-15T14:21:35.000Z",
"isMaintainer": false
}
],
"mainBotCommentID": 1660236738,
"ciResult": "pass"
} |
|
🔔 @gnapse @jgoz @smacpherson64 @AndrewLeedham — please review this PR in the next few days. Be sure to explicitly select |
|
Re-ping @gnapse, @jgoz, @smacpherson64: This PR has been out for over a week, yet I haven't seen any reviews. Could someone please give it some attention? Thanks! |
| customExpect(element).toHaveErrorMessage(/invalid time/i); | ||
| customExpect(element).toHaveErrorMessage(expect.stringContaining('Invalid time')); | ||
|
|
||
| customExpect(element).not.toBeInTheDOM(); |
There was a problem hiding this comment.
@kryops How come you removed all the .not test cases?
There was a problem hiding this comment.
The testing library types actually do not contain not, only the Jest types do.
These tests were only green because the types mapped every unkown property to any before, but I don't think they really made sense.
There was a problem hiding this comment.
Ah I see, because of the customExpect. Looks like the jest suite is still testing .not so this looks good :)
jgoz
left a comment
There was a problem hiding this comment.
Apologies for the delay. I've been meaning to chime in here that I was in the process of moving the typings for jest-dom back into the main repo: testing-library/jest-dom#511
I haven't put much thought into this particular change, but for jest-dom v6, it will need to be applied in the main repository too.
|
@kryops Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day! |
|
@jgoz thanks for the heads-up, closing in favor of testing-library/jest-dom#513 |
Please fill in this template.
npm test <package to test>.Select one of these and delete the others:
If changing an existing definition: