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

ReactTestUtils.mockComponent() supports function and forwardRef components #13192

Closed

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jul 11, 2018

I'm not very familiar with this method, or how it's used, so let me know if I'm overlooking anything 😄

@pull-bot
Copy link

Details of bundled changes.

Comparing: afd4649...3fcb666

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom-test-utils.development.js +1.2% +0.7% 42.86 KB 43.38 KB 11.92 KB 12 KB UMD_DEV
react-dom-test-utils.production.min.js 🔺+2.4% 🔺+0.9% 10.06 KB 10.31 KB 3.74 KB 3.78 KB UMD_PROD
react-dom-test-utils.development.js +1.2% +0.7% 42.58 KB 43.1 KB 11.85 KB 11.94 KB NODE_DEV
react-dom-test-utils.production.min.js 🔺+2.5% 🔺+0.8% 9.86 KB 10.1 KB 3.7 KB 3.73 KB NODE_PROD
ReactTestUtils-dev.js +1.3% +0.7% 41.43 KB 41.95 KB 11.23 KB 11.3 KB FB_WWW_DEV

Generated by 🚫 dangerJS

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2018

This is an unfortunate Facebook-ism that leaked into open source but I don't think anybody ever figured out how to use it there. It's been broken for functional components for a long time which further discouraged its use.

I think we should just deprecate and delete it in next major instead of trying to fix it. Unlike any other code in test renderer, it's tied to Jest (which is very odd but probably made sense in FB environment at some very early stage).

Internally, we can convert it to a standalone utility since it's not actually sharing any state with test utils.

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2018

I forgot: we already have an issue tracking this 😛
#11019

Other past discussions: #2499, #2660.

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's deprecate instead and convert internal callsites (which can then use this fix)

@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 11, 2018

Ah, I actually only encountered it because an FB engineer was trying to use it with a ref-forwarding component and was unable.

@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 11, 2018

I'll re-purpose this PR to be a deprecation, and suggest the FB engineer just pulls the code into an internal module.

@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 11, 2018

Closing in favor of #13193

@bvaughn bvaughn closed this Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants