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

Add the required "default" export mocking caveat to Jest migration guide. #1763

Closed
4 tasks done
vilbergs opened this issue Aug 1, 2022 · 5 comments
Closed
4 tasks done
Labels
enhancement New feature or request pr welcome

Comments

@vilbergs
Copy link

vilbergs commented Aug 1, 2022

Clear and concise description of the problem

It took me a while to realise that I needed to explicitly mock default module exports in vitest. Since this is a rather big change from the way we mock modules in jest, I think it's worth to mention this in the Jest migration guide.

This is well documented in https://vitest.dev/api/#vi-mock, but is easy to miss as it's not a part of the general mocking guide.

Suggested solution

Add a quick note about default export mocking in https://vitest.dev/guide/migration.html#migrating-from-jest. I think this will help people alot.

Alternative

Provide better error messages when people accidentally mock named exports with vi.mock. For example cannot read property 'get' of undefined, did you intend to mock the "default" export? See https://vitest.dev/api/#vi-mock for details.

Additional context

No response

Validations

@sheremet-va sheremet-va added enhancement New feature or request pr welcome labels Aug 1, 2022
@jereklas
Copy link
Contributor

jereklas commented Aug 9, 2022

This should be able to be closed as part of this PR? I guess I didn't update any documentation with that fix, do you want an update in either of the documents linked in this issue @sheremet-va ?

@sheremet-va
Copy link
Member

This should be able to be closed as part of this PR? I guess I didn't update any documentation with that fix, do you want an update in either of the documents linked in this issue @sheremet-va ?

Sure, let's update documentation.

@sheremet-va
Copy link
Member

sheremet-va commented Aug 9, 2022

This should be able to be closed as part of this PR? I guess I didn't update any documentation with that fix, do you want an update in either of the documents linked in this issue @sheremet-va ?

Also, I think I found a potential bug there:

It's not forbidden to export undefined from a module. Maybe it's better to check in? Or Reflect.has, or Object.hasOwn.

@jereklas
Copy link
Contributor

jereklas commented Aug 9, 2022

Or maybe just !target.hasOwnProperty(prop). I wont be able to get to this tonight, but can try putting up a PR fix along with some documentation updates tomorrow. I'll also add a test case for the explicit case where someone sets the export to undefined.

sheremet-va pushed a commit that referenced this issue Aug 11, 2022
…ocs (#1763) (#1830)

* handle undefined returns of module mocks, and update migration docs

* correct docs
@sheremet-va
Copy link
Member

Fixed #1830

@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request pr welcome
Projects
None yet
Development

No branches or pull requests

3 participants