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

Fix wrapped refs being called on every render #309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

felthy
Copy link

@felthy felthy commented Jun 18, 2019

I noticed that if I do something like this:

setRef = el => {
  if (el) {
    console.log('mounting', el)
  } else {
    console.log('unmounting')
  }
}

render() {
  const { onEnterViewport, onLeaveViewport } = this.props
  <Waypoint onEnter={onEnterViewport} onLeave={onLeaveViewport}>
    <div ref={this.setRef} />
  </Waypoint>
}

I see unmounting followed by mounting in the console every time my component renders. This is happening because the wrapper ref added in #278 changes on every render, so React calls it again (per Caveats with callback refs). This is a problem for me because I’m constructing and destroying a stateful video player in my setRef().

To fix this, I have:

  • added unit tests that verify the original behaviour;
  • added unit tests that fail in this case;
  • moved the ref-wrapping logic into the existing refElement instance method;
  • now the failing unit tests pass.

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

Successfully merging this pull request may close these issues.

1 participant