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

[Feature]: remove alised matchers #13164

Closed
G-Rath opened this issue Aug 23, 2022 · 6 comments · Fixed by #14632
Closed

[Feature]: remove alised matchers #13164

G-Rath opened this issue Aug 23, 2022 · 6 comments · Fixed by #14632

Comments

@G-Rath
Copy link
Contributor

G-Rath commented Aug 23, 2022

🚀 Feature Proposal

From jest-community/eslint-plugin-jest#1210 (comment)

Let's axe the following matchers:

expect(a).toBeCalled();
expect(a).toBeCalledTimes();
expect(a).toBeCalledWith();
expect(a).lastCalledWith();
expect(a).nthCalledWith();
expect(a).toReturn();
expect(a).toReturnTimes();
expect(a).toReturnWith();
expect(a).lastReturnedWith();
expect(a).nthReturnedWith();
expect(a).toThrowError();

in favor of their native counterparts:

expect(a).toHaveBeenCalled();
expect(a).toHaveBeenCalledTimes();
expect(a).toHaveBeenCalledWith();
expect(a).toHaveBeenLastCalledWith();
expect(a).toHaveBeenNthCalledWith();
expect(a).toHaveReturned();
expect(a).toHaveReturnedTimes();
expect(a).toHaveReturnedWith();
expect(a).toHaveLastReturnedWith();
expect(a).toHaveNthReturnedWith();
expect(a).toThrow();

Motivation

  • Removing duplicated matchers will return Jest's size a little
  • We've already identified the preferred matcher in each set
  • no-alias-methods provides a migration method, which can be automatically fixed
  • it's easy to extend the matchers Jest has, so folks can re-alias these themselves if they wish
  • eventually this would let external tools (like eslint-plugin-jest) shave off a little complexity i.e. by not having to check for two matchers

Example

No response

Pitch

Because the matchers are in the jest core.

@SimenB
Copy link
Member

SimenB commented Aug 24, 2022

Yeah, this should be fine to do for v30 👍

@SimenB SimenB added the Pinned label Aug 24, 2022
@EduardoSCosta
Copy link
Contributor

Hi, I would like to do this issue, can I?

And this matchers will be completely removed, right? Or just show a warning like Mathcher toBeCalled is beeing deprecated, use toHaveBeenCalled instead.

@SimenB
Copy link
Member

SimenB commented Aug 25, 2022

Thanks! Feel free to send a PR, but note that it won't be merged for some months 🙂 It'll land eventually, tho 😀

And yes, just remove it. We have a lint rule (no-alias-methods as linked in the OP) which can migrate users, so I don't think Jest needs to do anything special 🙂

@G-Rath
Copy link
Contributor Author

G-Rath commented Oct 20, 2023

Have opened DefinitelyTyped/DefinitelyTyped#67143 marking the matchers as deprecated

typescript-bot pushed a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue Oct 30, 2023
…Rath

* fix: mark alias matchers as deprecated

jestjs/jest#13164

* chore: remove trailing space

* test(jest): add uses of deprecated matchers

* chore(jest): double quotes not single quotes
@PiotrBryla
Copy link

In case someone wants a quick way to replace deprecated matchers in their tests, I've created a config file for a tool called Combyhttps://github.com/PiotrBryla/jest-30-aliases-update that allows for in-place replacement of many patterns in many files by running a single command.

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants