using shouldComponentUpdate instead of getSnapshotBeforeUpdate

[Enitoni]

Hi, I'm struggling a little with the new lifecycles. I have this component which passes down a style which will lock the scrollbar on the window without shifting content when applied on my root App element.

interface ScrollLockProps {
  locked: boolean,
  children: (style?: CSSProperties) => React.ReactNode
}

export class ScrollLock extends React.Component<ScrollLockProps> {
  private scrollY = 0

  public shouldComponentUpdate() {
    if (!this.props.locked) {
      this.scrollY = window.scrollY
    }

    return true
  }

  public componentDidUpdate() {
    if (!this.props.locked) {
      window.scrollTo(0, this.scrollY)
    }
  }

  public render() {
    const { locked, children } = this.props

    const style: CSSProperties | undefined = locked ? {
      position: "fixed",
      left: "0px",
      right: "0px",
      top: `-${this.scrollY}px`,
    } : undefined

    return children(style)
  }
}

The problem is I have to use shouldComponentUpdate to retrieve the scroll position, and this feels wrong to me as this lifecycle surely should only return a boolean and not have any side effects?

I can't use getSnapshotBeforeUpdate because render is called before it, which means the resulting style will be incorrect.

And of course I cannot use componentWillReceiveProps because it is deprecated. What is the correct way to go about this?

[bvaughn]

I don't think I understand the purpose of this component.

What causes your component to re-render when the window scroll position changes to ensure that its child stays "locked"?

Why do you call window.scrollTo() for the position that you've just read before rendering?

Would something like this work?

class ScrollLock extends Component {
  state = {
    // Initialize state to window's current scroll position.
    scrollY: window.scrollY
  };

  // Update state to match window's scroll position whenever it changes.
  handleScroll = event =>
    this.setState({
      scrollY: window.scrollY
    });

  componentDidMount() {
    window.addEventListener("scroll", this.handleScroll);
  }

  componentWillUnmount() {
    window.removeEventListener("scroll", this.handleScroll);
  }

  render() {
    const { locked, children } = this.props;

    // Read scroll position from state.
    const { scrollY } = this.state;

    const style = locked
      ? {
          position: "fixed",
          left: 0,
          right: 0,
          top: `-${scrollY}px`
        }
      : undefined;

    return children(style);
  }
}

[Enitoni]

The purpose of the component is to abstract away logic that I could write in the root App component itself.

  public render() {
    return (
      <ScrollLock locked={ modalStore.visibleModals.length > 0 }>
        { (style) => this.renderContent(style) }
      </ScrollLock>
    )
  }

What it does is prevent scrolling when locked is true by passing down styles. Think of it as another way to write overflow: hidden; on the body element, except it prevents the scrollbar from hiding so content doesn't shift.

The component itself works, but I was just wondering if I'm abusing React in any way and if it's possibly buggy.

[bvaughn]

What it does is prevent scrolling when locked is true by passing down styles.

I guess I still don't really understand how this component "prevents" the window from scrolling, or why you'd need to set the fixed-position top equal to window.scrollY rather than just some static value.

I also still don't understand what lets your component know to re-render when the window position changes, since you aren't listening to a "scroll" event.

but I was just wondering if I'm abusing React in any way and if it's possibly buggy.

Reading global values during render is not really safe. If your component is used within an async app, for instance, the external value (window.scrollY) might change between when the component is rendered and when it's updated in the DOM. Your component should only read values in props and state when rendering.

[Enitoni]

What it does is set this style on the App div like so:

This basically sets the element to 0 in height (because fixed does that), so there is nothing to scroll and then top offsets it by the amount scrolled so the page doesn't jump when switching between locked and not locked. It's a trick basically, but it's way better than overflow: hidden;

Hopefully you understand what the component does now.