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

Fix for pretty-format and react elements #7460

Closed
wants to merge 2 commits into from

Conversation

rickhanlonii
Copy link
Member

Summary

This PR fixes an issue with the pretty-format react plugin where it was only checking the $$typeof symbol for object element types. I found that function element types are possible with the $$typeof checks, so I moved those checks to before the typeof type === function check

The practical change is that in some scenarios, instead of printing

<Component />

We will now print

<ForwardRef(Foo) />

Test plan

I actually can't figure out how to write a test for this issue in our repo, but I did apply the change manually in https://github.com/rajivshah3/rn-broken-snapshot/tree/broken-example, where I discovered the issue, to confirm the fix

I suspect it's related to some combination of jest, react, react-native, and react-test-renderer

@thymikee
Copy link
Collaborator

thymikee commented Dec 4, 2018

This is also very dependent on RN version – <Component /> usually signs lack of proper displayName provided for ForwardedRefs. Just make sure with React team that we can safely check functions same as objects.

@SimenB
Copy link
Member

SimenB commented Dec 4, 2018

Can we use https://www.npmjs.com/package/react-is?

@SimenB
Copy link
Member

SimenB commented Feb 14, 2019

#7895

@SimenB SimenB closed this Feb 14, 2019
@SimenB
Copy link
Member

SimenB commented Mar 7, 2019

Ergh, the fix this PR attempt to provide isn't solved... Not sure why I closed this.

We now use react-is at least, so it should be cleaner. This needs a test, though 🙂

@SimenB SimenB reopened this Mar 7, 2019
@SimenB
Copy link
Member

SimenB commented Nov 9, 2019

@rickhanlonii any plans for this?

@rickhanlonii
Copy link
Member Author

Probably will not be able to look at this until next year

@SimenB
Copy link
Member

SimenB commented Jul 27, 2020

@rickhanlonii it's a new year! 😀

@rickhanlonii
Copy link
Member Author

Wow, @SimenB I literally just started fixing this because it's a bigger issue on the React main branch, thanks for bumping and reminding me that I already fixed it. I'll get this in.

@SimenB
Copy link
Member

SimenB commented Jul 27, 2020

Perfect! 😀 We got #10120 from a Ghost as well, should it be closed by this?

@rickhanlonii
Copy link
Member Author

Actually I'm going to close this, the bug here was in the way we were doing mocks in React Native and was fixed by facebook/react-native@4b935ae

@SimenB
Copy link
Member

SimenB commented Jul 29, 2020

👍

@rickhanlonii rickhanlonii deleted the rh-fix-forward-ref branch July 29, 2020 14:51
@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 11, 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.

4 participants