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

innerRef support for withTheme() HOC #591

Closed
evan-scott-zocdoc opened this issue Mar 17, 2017 · 12 comments
Closed

innerRef support for withTheme() HOC #591

evan-scott-zocdoc opened this issue Mar 17, 2017 · 12 comments

Comments

@evan-scott-zocdoc
Copy link
Contributor

evan-scott-zocdoc commented Mar 17, 2017

Version

1.4.4

Reproduction

https://www.webpackbin.com/bins/-KfSbgVyPqFmY7yJYVj0

Steps to reproduce

Style a withTheme()-wrapped component and try to use the innerRef callback to grab the wrapped component instance.

Expected Behavior

I would expect the innerRef callback to resolve to the deeply nested child.

Actual Behavior

The withTheme HOC instance is returned by innerRef() and there's no way to get the wrapped child component.

@evan-scott-zocdoc
Copy link
Contributor Author

evan-scott-zocdoc commented Mar 17, 2017

Ugh accidentally overwrote my bin, hang on. fixed

@brunolemos
Copy link
Member

@evan-scott-zocdoc Just to make sure...

This is the current output:

"this.child instanceof Child === false"
"this.child instanceof ThemedChild === true"
"this.child instanceof StyledThemedChild === false"

and you were expecting this:

"this.child instanceof Child === true"
"this.child instanceof ThemedChild === false"
"this.child instanceof StyledThemedChild === false"

Is that correct?

@evan-scott-zocdoc
Copy link
Contributor Author

evan-scott-zocdoc commented Mar 18, 2017 via email

@brunolemos
Copy link
Member

brunolemos commented Mar 18, 2017

I don't think it's related to withTheme because if you replace the following line:

-const ThemedChild = withTheme(Child);`
+const ThemedChild = styled(Child)``;

The output will be all false.
The innerRef won't even be called.

@evan-scott-zocdoc
Copy link
Contributor Author

Ah the example should probably be tweaked, as you can't take a ref on a SFC. I'll do that now.

@evan-scott-zocdoc
Copy link
Contributor Author

@brunolemos updated the example, your change will now reflect Child as the constructor as expected.

@mxstbr
Copy link
Member

mxstbr commented Apr 1, 2017

This should definitely be fixed! Thanks for the reproduction.

@kitten kitten added this to the v2.0 milestone Apr 19, 2017
@kitten
Copy link
Member

kitten commented Apr 19, 2017

This PR should fix this 😄 #710

@evan-scott-zocdoc @mxstbr

@evan-scott-zocdoc
Copy link
Contributor Author

Woot!

@kitten
Copy link
Member

kitten commented Apr 20, 2017

All done 😸

@kitten kitten closed this as completed Apr 20, 2017
@denkristoffer
Copy link

denkristoffer commented Apr 5, 2018

I'm seeing this behaviour again with SC 3.2.5 (https://codesandbox.io/s/056wj1zmw), possibly because of #1414 ? Or am I doing something wrong here? I can't access the bin in OP to compare the code.

@kitten
Copy link
Member

kitten commented Apr 5, 2018

@denkristoffer That happens because your InputWithRef component (which shouldn't be necessary in the first place 😉) tries to use the ref prop, which is a special prop and thus not accessible to you. tl;dr you can't pass ref and expect to receive it in the component. That's why we call it innerRef. Renaming it fixes
the problem: https://codesandbox.io/s/72m8xxy121

Btw it's not preferable to post comments on closed/stale/old issues (this issue is all of those three things 😅) It's easier to create a new issue or ask for help on Spectrum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants