react / react/react-native

<VirtualizedList renderScrollComponent> should preserve child elements

Open
#40,740 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Component: VirtualizedList Needs: Repro Needs: Triage :mag: Never gets stale
Dominant language
C++
Stars
127k
Forks
25.3k
Avg merge
1d 23h
Merged PRs (30d)
4

Description

Description

It's extremely confusing that this works:

<VirtualizedList
  renderScrollComponent={props =>
    <ScrollView {...props} />
  }
  // ...
/>

but this doesn't:

<VirtualizedList
  renderScrollComponent={props =>
    <View>
      <ScrollView {...props} />
    </View>
  }
  // ...
/>

The reason appears to be that the third cells argument to cloneElement call here (which in this case would be passed a <View>) overrides the children of that top-level element. So the ScrollView inside gets lost.

What I believe should happen instead is that scrollProps should include children: cells. Then it's up to your renderScrollComponent function to decide what to do with it.

That would be a breaking change but it seems a lot more in line with how React components are supposed to work? It's bad that wrapping a top-level component completely breaks the logic. (I spent an hour debugging this because I couldn't believe a built-in RN component would do this :). It's possible that I badly misunderstood something, so apologies if that's the case.

React Native Version

0.72.5

Output of npx react-native info
info Fetching system and libraries information...
System:
  OS: macOS 13.2.1
  CPU: (10) arm64 Apple M1 Max
  Memory: 12.29 GB / 64.00 GB
  Shell:
    version: 5.8.1
    path: /bin/zsh
Binaries:
  Node:
    version: 18.17.1
    path: /usr/local/bin/node
  Yarn:
    version: 1.22.19
    path: /usr/local/bin/yarn
  npm:
    version: 9.6.7
    path: /usr/local/bin/npm
  Watchman:
    version: 2023.01.30.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.11.3
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 22.4
      - iOS 16.4
      - macOS 13.3
      - tvOS 16.4
      - watchOS 9.4
  Android SDK: Not Found
IDEs:
  Android Studio: 2022.1 AI-221.6008.13.2211.9514443
  Xcode:
    version: 14.3.1/14E300c
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: javac 18
    path: /usr/bin/javac
  Ruby:
    version: 2.7.6
    path: /Users/dan/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.72.5
    wanted: 0.72.5
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false
Steps to reproduce

see earlier

Snack, screenshot, or link to a repository

this is about API design

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/virtualized-lists/Lists/VirtualizedList.js around the scrollProps construction and cloneElement call linked in the issue. Reproduce the renderScrollComponent examples, then determine whether preserving the wrapped child can be covered by an appropriate regression test and whether the proposed children behavior is acceptable as an API change.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react-native
Domain
mobile
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.