Scrollable broken with React 18 automatic batching

[iAmNathanJ]

Issue summary

From what I can tell, the <Scrollable> component is broken with automatic batching introduced in React 18. This only seems to affect mobile devices with momentum scrolling. I haven't been able to reproduce on desktop.

Expected behavior

A smooth scrolling experience

Actual behavior

Stuttered scrolling on mobile devices (iOS Safari, Firefox)

Screen Recordings

Real device shows momentum scrolling is halted and scroll position can jump unpredictably:

Video.mov

Simulator shows constant scroll jank:

17-51-hlclp-7pe3d.6.mp4

Steps to reproduce the problem

Use react@18. See reduced test cases below.

Reduced test case

Visit these links with on an iOS device. I've only tested (Safari and Firefox).

• 👎🏻 Scrollable with React 18
• 👍🏻 Scrollable with React 17

Specifications

• Are you using the React components? (Y/N): Yes
• Polaris version number: 10.7.0
• Browser: Safari
• Device: iPhone
• Operating System: iOS

  Binaries:
    Node: 16.18.0 - ~/.nvm/versions/node/v16.18.0/bin/node
    Yarn: 1.22.15 - /usr/local/bin/yarn
    npm: 8.19.2 - ~/.nvm/versions/node/v16.18.0/bin/npm
    Watchman: 4.9.0 - /usr/bin/watchman
  npmPackages:
    @shopify/polaris: 10.7.0 => 10.8.0 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 

---

Potential fixes
I'd be happy to open a PR to fix this, but I assume the Polaris team would need to weigh in on the best approach.

From what I can tell, the conflict with batching state updates is only related to the setting scrollTop for the scrollable container inside componentDidUpdate. But why that is done in the Scrollable component is not clear to me. It seems we use the DOM as the source of truth to update state, but then turn around in componentDidUpdate and use state as the source of truth to update the DOM 🤔.

Some options I've observed that remedy the issue:

• Wrap Scrollable's setState in ReactDOM.flushSync
  • This is probably not a great option because the react-dom is a peer dependency and polaris supports v16-18. Does flushSync work with past react versions?
• Remove componentDidUpdate in Scrollable - no more scroll jacking
  • Need to know why it's there in the first place, all tests still pass after removing it

🗒️ Additional Notes
Related to the logic in componentDidUpdate, an issue sometimes occurs which causes the scroll to stick when scrolled to the very bottom of the container and starting an upward scroll. When the scroll sticks, it can cause a native "pull down to refresh" gesture for the page. Leading to a frustrating reload.

195644309-da004127-de6b-4895-adc8-21112beeddd8.mov