-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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 issue finding nested SFCs by constructor in mount #274
Fix issue finding nested SFCs by constructor in mount #274
Conversation
@@ -19,6 +19,10 @@ export function internalInstance(inst) { | |||
inst[internalInstanceKey(inst)]; | |||
} | |||
|
|||
export function isFunctionalComponent(inst) { | |||
return inst.constructor.name === 'StatelessComponent'; |
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.
will inst.constructor
always exist?
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.
Provided what is passed to the function is an instance, yes, I think it will have a constructor.
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.
let's maybe fence this with inst && inst.constructor &&
anyway? I intend on using this in several places, and it seems possible for us to pass null in at some point...
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.
@lelandrichardson @ljharb I've constructed a fence!
7cd8f3d
to
558d6e2
Compare
I've updated this with a different (correct-er) solution and another test. |
@@ -249,8 +255,7 @@ function findAllInRenderedTreeInternal(inst, test) { | |||
const internal = internalInstance(inst); | |||
return findAllInRenderedTreeInternal(internal, test); | |||
} | |||
|
|||
const publicInst = inst.getPublicInstance(); | |||
const publicInst = inst.getPublicInstance() || inst._instance; |
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.
should the _instance
stuff be moved into getPublicInstance
?
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.
I believe .getPublicInstance
is a private method in React itself, which always returns null
for SFCs, thus we fall back to ._instance
.
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.
ah ok, if it's a react method then we certainly can't fix that :-) LGTM
558d6e2
to
1a43f27
Compare
@iancmyers think you can rebase? |
This fixes an issue where calling `.find` with an SFC constructor/ function would fail to return instances of the the SFC in the tree. Fixes enzymejs#262
1a43f27
to
476a9cc
Compare
Fix issue finding nested SFCs by constructor in mount
This fixes an issue where calling
.find
with an SFC constructor/function would fail to return instances of the the SFC in the tree.This seems like a round-about way of getting at this. Is there a better way?
Fixes #262