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 finding forwardRef itself #1684

Closed
wants to merge 5 commits into from
Closed

Conversation

cyan33
Copy link

@cyan33 cyan33 commented Jun 20, 2018

This PR is built on top of @jquense 's #1592

It fixes when the wrapper.find method cannot find the forwardRef itself. The reason this is happening is that React.forwardRef returns a type that is unknown to enzyme. The return type looks like this:

{$$typeof: Symbol.for('react.forward_ref'), render: [Function]}

What we really want to search is the thing wrapped inside of forwardRef, so one way to do this is to call the render method and use the return element to re-build a predicate based on that.

@@ -3,6 +3,11 @@ import PropTypes from 'prop-types';

/* eslint react/forbid-prop-types: 0 */

// TODO: use react-is?
const specialType = PropTypes.shape({
Copy link
Contributor

Choose a reason for hiding this comment

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

should use react-is

if (typeOf(selector) === ForwardRef) {
// re-build the predicate based on what is wrapped by forwardRef
// rather than the forwardRef itself
return buildPredicate(selector.render().props);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the right way to do this but maybe i'm misunderstanding. We should copy waht react-test-renderer does and this code should go in the react-16-adapter

Copy link
Author

Choose a reason for hiding this comment

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

Actually I'm not sure either. Maybe the root cause of this is that enzyme is not generating a corresponding element for forwardRef and we shouldn't defer searching to the next level here. I'll look into react-test-renderer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah the selector module is used for all adapters, so the node type should be opaque in this module.

@ljharb
Copy link
Member

ljharb commented Jun 29, 2018

@jquense are you able to pull this PR into yours, such that we don't have to have stacked PRs?

@jquense
Copy link
Contributor

jquense commented Jun 29, 2018

I'll update my PR. I think someone actually already PR'd My PR with the correct fix

@ljharb
Copy link
Member

ljharb commented Jun 29, 2018

(I'll leave this open and sync the two of them myself when the first one's ready to merge)

@cyan33
Copy link
Author

cyan33 commented Jun 29, 2018

Actually this PR's change is not correct as @aweary and @jquense pointed out that the change should not be in enzyme core. I've updated my fix in the comment of @jquense's PR. I'll close it for now.

@cyan33 cyan33 closed this Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants