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

refs, dom elements, and grid instance api #1418

Closed
jedwards1211 opened this issue Sep 6, 2019 · 0 comments
Closed

refs, dom elements, and grid instance api #1418

jedwards1211 opened this issue Sep 6, 2019 · 0 comments
Labels

Comments

@jedwards1211
Copy link
Contributor

jedwards1211 commented Sep 6, 2019

@bvaughn react-virtualized is an example of the uncommon case of using refs to get access to a component instance to call methods on it. Most of the time people just use refs to get a DOM element.

What do you think about providing access to these instance methods without using refs?

For example,

class Grid {
  ...
  componentDidUpdate() {
    const {getApi} = this.props
    if (getApi) {
      const {
        getOffsetForCell,
        getTotalRowsHeight,
        getTotalColumnsWidth,
        handleScrollEvent,
        measureAllCells,
        recomputeGridSize,
        scrollToCell,
        scrollToPosition,
      } = this
      getApi({
        getOffsetForCell,
        getTotalRowsHeight,
        getTotalColumnsWidth,
        handleScrollEvent,
        measureAllCells,
        recomputeGridSize,
        scrollToCell,
        scrollToPosition,
      })
    }
  }
}

I've been thinking about how ref forwarding is really awkward - It changed the possible types of react elements, it doubles the height of many component trees, and it requires more work on the developer's part, and caused a lot of disruption in the ecosystem (e.g. it broke react-redux for a short while).

All of this made me realize it would be vastly better if refs are automatically forwarded to the first DOM element (with forwardRef being used only to control which child of a React.Fragment the ref gets forwarded to).

This would mean dropping support for refs to component instances, meaning libraries like this would have to use custom props instead of refs to pass API methods back to the parent. But I think it would be justified for the following reasons:

  • Refs to component instances are not very common AFAIK
  • Refs to component instances give access to things users shouldn't really have access to (component state, setState, componentDidUpdate, render, etc)
  • Using ref forwarding with a class-based component necessitates putting it inside two wrapper components...not elegant at all
  • If someone wants to get the root DOM element for a <Grid> for whatever reason, they have to figure out an awkward workaround (containerProps={{ref: ...}} I think?)
  • I think people will increasingly expect a ref on any component to just get them a DOM element

I've been thinking about proposing this in react itself but I wanted to get your take on this first since it would directly impact libraries like RV.

@github-actions github-actions bot added the Stale label Sep 17, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant