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

RFC: Mock subtype #6011

Closed
wants to merge 3 commits into from
Closed

RFC: Mock subtype #6011

wants to merge 3 commits into from

Conversation

mjesun
Copy link
Contributor

@mjesun mjesun commented Apr 17, 2018

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 its react.component value when duck typing by the new static methods. It adds a default implementation that does this.state = {}, and mocks the default value of gDSFP to null.

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.

flarnie and others added 2 commits April 15, 2018 18:36
**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.
Copy link
Member

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') {
Copy link
Member

@SimenB SimenB Apr 17, 2018

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

@cpojer
Copy link
Member

cpojer commented Apr 17, 2018

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 ).

@mjesun
Copy link
Contributor Author

mjesun commented Apr 17, 2018

Makes sense. Abandoning then!

@mjesun mjesun closed this Apr 17, 2018
@mjesun mjesun deleted the mock-subtype branch April 17, 2018 16:42
@github-actions
Copy link

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.
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 May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants