-
-
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
RFC: Mock subtype #6011
RFC: Mock subtype #6011
Conversation
**what is the change?:** Soon folks will be adopting the new 'getDerivedStateFromProps' in their React components, and re-running existing tests. If those components are mocked in existing tests, then React will throw errors currently. Why? Because React validates that any component defining this method has state and returns the right thing from 'gDSFP', and when an auto-mock is created it doesn't fit these invariants. Talking through this with @mael and @mjesun we determined an easy solution is to add a small conditional to either blacklist mocking functions with this name, so that automocks just don't have this method and the invariant is never checked in React. An alternative solution would be to make the mocked 'gDSFP' method return 'null' and define 'state' on the mocked component. **why make this change?:** Let's work together to make Jest and React work together incredibly smoothly, like a sea of butter. ^_^ **test plan:** This test will throw errors right now - tagging @mjesun to tweak Jest so that the new test will pass correctly. (Flarnie will insert screenshot of the errors thrown)
@@ -0,0 +1,3806 @@ | |||
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. |
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 shouldn't be here, just add the examples to workspaces
@@ -600,6 +612,15 @@ class ModuleMockerClass { | |||
mock.prototype.constructor = mock; | |||
} | |||
|
|||
if (metadata.subtype === 'react.component') { |
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.
baking this into jest-mock
itself seems hairy. Is it possible to somehow extend jest-mock
with plugins? So getSubtype
existed, but it's possible to plug in specific subtypes, like React. Then Jest could extend it, but not couple jest-mock
to React
I'm quite heavily against this magic change to Jest's mocking package. We do not generally add framework version specific code to Jest to make things easier. I would much rather prefer a hack similar to what React has already been doing in the past to support automocking better (Example: https://twitter.com/dan_abramov/status/980510354810732544 ). |
Makes sense. Abandoning then! |
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. |
With the upcoming version of React, and the new
getDerivedStateFromProps
method, new restrictions have been added to the way components should behave in React (more info on #6000, in which I based my commit).@flarnie introduced a test case where two warnings get triggered when automocking a component, which is not nice.
This PR adds a specific
subtype
concept to the metadata used to build the mock, and uses itsreact.component
value when duck typing by the new static methods. It adds a default implementation that doesthis.state = {}
, and mocks the default value ofgDSFP
tonull
.I'm not fully happy with the PR, but TBH it looks like a trade-off between introducing a system to fully configure mocks after creating them and letting all React tests pseudo-break.