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

Ref's don't have all the properties one would expect. #14145

Closed
cinnamennen opened this issue Nov 8, 2018 · 2 comments
Closed

Ref's don't have all the properties one would expect. #14145

cinnamennen opened this issue Nov 8, 2018 · 2 comments

Comments

@cinnamennen
Copy link

Do you want to request a feature or report a bug?
Bug
What is the current behavior?
With a class like

class Parent extends React.Component {
  constructor(props) {
    super(props);
    this.tableRef = createRef();
  }

  render() {
    return (
      <div>
        <Element1
          inputRef={this.tableRef}
        />
        <Element2
          style={{ maxHeight: this.tableRef.current
                .getBoundingClientRect().height }}
        />
      </div>
    );
  }
}

class Element1 extends React.Component {
  render() {
    return (
      <Table ref={this.props.inputRef}>
        {/*Contents*/}
      </Table>
    );
  }
}

getBoundingClientRect() doesn't work. Instead you must resort to findDOMNode, like

<Element2
          style={{ maxHeight: findDOMNode(this.tableRef.current)
                .getBoundingClientRect().height }}
/>

What is the expected behavior?
Given that refs are supposed to negate the need to use findDOMNode, this seems like a weak spot.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
react 16.4.2, Chome

@markerikson
Copy link
Contributor

markerikson commented Nov 8, 2018

What is <Table> ? Unless it's using forwardRef internally, putting a ref on it won't hand back a DOM node - it'll handle back the Table class instance.

This looks like a logic error in use of refs to me.

Also, the use of this.tableRef.current directly in render() looks like it would crash the first time through anyway, because tableRef.current will be null the first time the component renders.

@Jessidhia
Copy link
Contributor

Even if you're not using forwardRef, what you did with inputRef is really close to the right thing. You just need to also do it in Table.

(Note, semantic-ui-react, which happens to have a Table component, has no support for refs at all in current releases; they're working on a forwardRef PR)

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

3 participants