-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Imported jest in mock #13188
Imported jest in mock #13188
Conversation
Normally this would also transform the
Yes (I think). Caveat is it should only be hoisted within its function scope, not (necessarily) to the top
Yeah, that should be fine. |
@nicolo-ribaudo are there more work you'd want to do here, or is it ready to merge? 🙂 |
This is ready, but I'll follow up with another PR for those small simplifications :) |
Awesome, thanks! |
CHANGELOG.md
Outdated
- `[jest-config]` [**BREAKING**] Make `snapshotFormat` default to `escapeString: false` and `printBasicPrototype: false` ([#13036](https://github.com/facebook/jest/pull/13036)) | ||
- `[jest-config]` [**BREAKING**] Remove undocumented `collectCoverageOnlyFrom` option ([#13156](https://github.com/facebook/jest/pull/13156)) | ||
- `[jest-environment-jsdom]` [**BREAKING**] Upgrade to `jsdom@20` ([#13037](https://github.com/facebook/jest/pull/13037), [#13058](https://github.com/facebook/jest/pull/13058)) | ||
- `[jest-environment-jsdom]` **BREAKING**] Upgrade to `jsdom@20` ([#13037](https://github.com/facebook/jest/pull/13037), [#13058](https://github.com/facebook/jest/pull/13058)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was probably not intended
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried editting on mobile, crashed chrome 😅 should probably move old releases to a separate changelog
Note to self: trying to update a huuuuge changelog on mobile (while getting tattooed) does not work at all 😅 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This PR:
jest.*
refs to importedjest
in non-top-level contextsjest.*
refs inside hoisted code to the "lazy" versionFixes #12422
Test plan
Tests added
I have two questions:
This test doesn't make sense to me, since
import
s are hoisted and thus thejest
import is already hoisted, even without any transform:What's the reason to transform it?
Can we always transform
jest
to_getJestObj
, so that we don't have to check if it's hoisted or not and we can simplify the plugin?Since
require
is cached by default, can we userequire('@jest/globals').jest
directly instead of_getJestObj()
?