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

[V2] innerRef doesn't work with styled(Component) #618

Closed
kopax opened this issue Mar 24, 2017 · 7 comments
Closed

[V2] innerRef doesn't work with styled(Component) #618

kopax opened this issue Mar 24, 2017 · 7 comments

Comments

@kopax
Copy link
Contributor

kopax commented Mar 24, 2017

I am trying to set the ref for a <Input /> styled components.

The component has been created with const Input = styled(InputUnstyled).

I have the following react app:

function _objectWithoutProperties(obj, keys) { var target = {}; for (var i in obj) { if (keys.indexOf(i) >= 0) continue; if (!Object.prototype.hasOwnProperty.call(obj, i)) continue; target[i] = obj[i]; } return target; }

class InputUnstyled extends React.Component {
  render() {
      var _props = this.props,
          isOpen = _props.isOpen,
          rest = _objectWithoutProperties(_props, ["isOpen"]);

      return React.createElement("input", rest);
  }
}

const Input = styled(InputUnstyled)`
  font-size: 1.5em;
  text-align: center;
  color: palevioletred;
`;

const Wrapper = styled.section`
  padding: 4em;
  background: papayawhip;
`;

export default class App extends React.Component {
  
  constructor(props) {
    super(props);
    this.handleIsOpen = this.handleIsOpen.bind(this);
    this.state = { isOpen: false };
  }
  
  componentDidUpdate() {
     this.handleFocus(); 
  }
  componentDidMount() {
     this.handleFocus(); 
  }
  
  handleIsOpen() {
     this.setState({ isOpen: !this.state.isOpen }); 
  }
  
  handleFocus() {
     this.search.focus(); 
  }
  
  render() {
    return (
      <Wrapper>
        <button type="button" onClick={this.handleIsOpen}>Rerender</button>
        <Input type="text" isOpen={this.state.isOpen} innerRef={(search) => { this.search = search }} />
      </Wrapper>
    );
  }
}

Version

2.0.0-8

Reproduction

https://www.webpackbin.com/bins/-Kg-9xQ8QPTOnSFLmOOt

Steps to reproduce

  1. Open the WebpackBin link.
  2. Open your developer console.
  3. Click on the button to trigger this.search.focus().

It will through an error in your developer console.

Expected Behavior

The <Input /> element should receive the :focus.

Actual Behavior

Throw error this.search.focus is not a function

@kopax kopax changed the title [V2] innerRef doesn't work doesn't work with styled(Component) [V2] innerRef doesn't work with styled(Component) Mar 24, 2017
@kopax
Copy link
Contributor Author

kopax commented Mar 25, 2017

I've just notice this bug was also present in 1.4.4 See #592

@kitten
Copy link
Member

kitten commented Mar 25, 2017

This is not a bug, but due to how the "innerRef" logic works. Once it's in the input props, it will pass it innerRef as ref to the underlying element , I.e. The unstyled component in your example.

Thus the component that renders Input will receive a ref, but it won't be the element, but the unstyled component itself. If you want the element ref then either add your own ref prop and set it on the component instance, so that you can access it with its ref, or use findDOMNode.

@kitten kitten closed this as completed Mar 25, 2017
@kopax
Copy link
Contributor Author

kopax commented Mar 25, 2017

@philpl can you please elaborate I didn't really get it.

I have read here the recommendation.

add your own ref prop and set it on the component instance, so that you can access it with its ref

Could you please show a small example in the best case update the webpackbin to see how you would handle the problem ?

or use findDOMNode.

There is a warning if you use findDOMNode. You are not supposed to use findDOMNode as they won't work (haven't tried) in react-native. Styled-components is targeting full compatibility with react-native so it can't be the best solution for our components.

Is there any way to pass the innerRef through in case it's a styled components ? There is no reason to use innerRef if you target the styled components.

I suggest to reopen this. Or the doc must be updated at some point.

@kitten
Copy link
Member

kitten commented Mar 25, 2017

@kopax So the thing is, that we can't pass it through. We would have to expose innerRef and the wrapped component would have to accept innerRef and pass it as ref to one of the children. Otherwise we'd have to check what it renders, clone it, and pass that on, which is not even good in terms of correctness, since the user might render more than one element anyway.

Right now we directly pass innerRef as ref and thus you'll see your component instance as a ref, and not a DOM element.

Let's look at your example:

<Input type="text" isOpen={this.state.isOpen} innerRef={(search) => { this.search = search }} />

search in the callback will be the rendered instance of InputUnstyled.

If you want to get a DOM ref you could do:

class InputUnstyled extends React.Component {
  render() {
    const { isOpen, ...rest } = this.props
    return <input ref={input => this.ref = input} {...rest} />
  }
}

And then accept it like this:

<Input type="text" isOpen={this.state.isOpen} innerRef={(search) => { this.search = search.ref }} />

If we'd decide to pass on input ref, then you'd have to do:

class InputUnstyled extends React.Component {
  render() {
    const { isOpen, innerRef, ...rest } = this.props
    return <input ref={innerRef} {...rest} />
  }
}

Which works, until you render multiple elements. At that point, you'd be up to yourself, with no way to pass anything sensible accept maybe innerRef(this), which is weird.

We could also cloneElement in the wrapper which is IMHO ugly and constricts the user in such a way that it's impossible to escape the default behaviour.

I agree that we need to add some docs for this, but I don't think that there's a better default behaviour for this use-case.

@kopax
Copy link
Contributor Author

kopax commented Apr 3, 2017

Thanks for the detailed explanation it makes a bit more sens.

This should be added to the documentation.

@magnumnighthawk
Copy link

@philpl Thanks .. That worked for me. In my case, I was trying to get the reference of a third party component, react-custom-scrollbar. I managed to do it in the following way

import Scrollbars from 'react-custom-scrollbars';

export const StyledScroll = styled(({ _ref, ...rest }) =>  <Scrollbars ref={_ref} {...rest} />)`
  /* Styles */
`;

And invoked it like this

<StyledScroll _ref={(el) => { this.scrollbar = el; }} />

@lucasconstantino
Copy link

Couldn't we just check if the underlying component is a styled component itself, and in that scenario pass down innerRef instead of ref?

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

No branches or pull requests

4 participants