Using setState & this.props in onLayout callback of SectionList throws an error - "this.props" should not be accessed during state updates

[priyeshshah11]

New Version

0.71.2

Old Version

0.70.4

Build Target(s)

iOS 11.0 & above

Output of react-native info

System:
    OS: macOS 12.2.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 98.67 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    Yarn: 1.22.18 - /usr/local/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
    Watchman: Not Found
  Managers:
    CocoaPods: 1.11.3 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 15.4, macOS 12.3, tvOS 15.4, watchOS 8.5
    Android SDK: Not Found
  IDEs:
    Android Studio: 2021.3 AI-213.7172.25.2113.9123335
    Xcode: 13.3.1/13E500a - /usr/bin/xcodebuild
  Languages:
    Java: 16.0.2 - /usr/bin/javac
  npmPackages:
    @expensify/react-native:  0.71.2-alpha.2 
    @react-native-community/cli: Not Found
    react: 18.2.0 => 18.2.0 
    react-native: Not Found
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Issue and Reproduction Steps

Problem

Using setState & this.props in onLayout callback of SectionList throws an error

Error

"this.props" should not be accessed during state updates

Reproduction steps

Create a MySectionList class component that uses a SectionList with below props & an onLayout callback where you call a local class method before calling the passed down onLayout callback to your class component.

 /**
   * Scrolls to the focused index within the SectionList
   *
   * @param {Number} index
   * @param {Boolean} animated
   */
  scrollToIndex(index, animated = true) {
      const option = this.state.allOptions[index];
      if (!this.list || !option) {
          return;
      }

      const itemIndex = option.index;
      const sectionIndex = option.sectionIndex;

      // Note: react-native's SectionList automatically strips out any empty sections.
      // So we need to reduce the sectionIndex to remove any empty sections in front of the one we're trying to scroll to.
      // Otherwise, it will cause an index-out-of-bounds error and crash the app.
      let adjustedSectionIndex = sectionIndex;
      for (let i = 0; i < sectionIndex; i++) {
          if (_.isEmpty(lodashGet(this.props.sections, `[${i}].data`))) {
              adjustedSectionIndex--;
          }
      }

      this.list.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated});
  }

<SectionList
    ref={el => this.list = el}
    optionHoveredStyle={this.props.optionHoveredStyle}
    onSelectRow={this.selectRow}
    sections={this.props.sections}
    focusedIndex={this.state.focusedIndex}
    selectedOptions={this.props.selectedOptions}
    canSelectMultipleOptions={this.props.canSelectMultipleOptions}
    hideSectionHeaders={this.props.hideSectionHeaders}
    headerMessage={this.props.headerMessage}
    boldStyle={this.props.boldStyle}
    showTitleTooltip={this.props.showTitleTooltip}
    isDisabled={this.props.isDisabled}
    shouldHaveOptionSeparator={this.props.shouldHaveOptionSeparator}
    onLayout={() => {
        this.scrollToIndex(this.state.focusedIndex, false);

        if (this.props.onLayout) {
            this.props.onLayout();
        }
    }}
    contentContainerStyles={shouldShowFooter ? undefined : [this.props.safeAreaPaddingBottomStyle]}
/>

CC: @NickGerleman

[NickGerleman]

The invariant violation here is that VirtualizedList is accessing this.props during scrollTo at the same time as updating state, which is potentially dangerous, since it is reading a copy which may be out of date. This triggers an invariant in 0.71, where we are not permissive of this.

Could you post the full callstack when this happens? I would expect to see more VirtualizedList code happening lower in the stack. It seems most likely that VirtualizedList is calling onViewableItemsChanged while updating its own state, so trying to scroll it before the state update has finished. But it would be good to get confirmation of that (and then the fix would be for VirtualizedList to defer its own callbacks).

In the meantime I would recommend deferring any imperative methods until after the onViewableItemsChanged callback.

[NickGerleman]

#36340 is landing and should likely fix your issue.

Feel free to make a pick request for this, but you can otherwise workaround this by avoiding scrolling in callbacks for viewability changes (semi-related, calling onLayout in response to something not a layout event will probably end up confusing the next person to work on that code 😅).