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(Android): incorrect childCount in removeViewAt when using flatlist on fabric #2307

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

alduzy
Copy link
Member

@alduzy alduzy commented Aug 19, 2024

Description

This PR intents to fix the crash when navigating back from a screen with FlatList on the new architecture. The crash was caused by miscalculated childCount of the list.
Earlier on I found out that setting the removeClippedSubviews option to false (defaults to true on Android) in the FlatList fixes the problem.

This PR is rather a quick fix with an extra condition, that adds simple views in place of the miscalculated ones in startTransitionRecursive function if there's a FlatList with removeClippedSubviews option set.

Fixes #2282.

Changes

  • added Test2282.tsx repro
  • added extra condition in startTransitionRecursive function

Test code and steps to reproduce

  • added Test2282.tsx repro

Checklist

  • Ensured that CI passes

Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an ugly workaround but let's stick with it for now.

@alduzy alduzy merged commit c47ad84 into main Aug 20, 2024
4 checks passed
@alduzy alduzy deleted the @alduzy/child-count-fabric branch August 20, 2024 13:49
@lluisandreu
Copy link

Hello @kkafar, are you going to create a release soon with this fix? Thanks

@kkafar
Copy link
Member

kkafar commented Sep 13, 2024

Planning a stable 3.35.0 in the beginning of the next week. But don't hold me to it, it will be ready when it will be ready (Cyberpunk vibes unintended)

ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…t on fabric (software-mansion#2307)

## Description

This PR intents to fix the crash when navigating back from a screen with
FlatList on the new architecture. The crash was caused by miscalculated
`childCount` of the list.
Earlier on I found out that setting the
[removeClippedSubviews](https://reactnative.dev/docs/flatlist#removeclippedsubviews)
option to false (defaults to true on Android) in the FlatList fixes the
problem.

This PR is rather a quick fix with an extra condition, that adds simple
views in place of the miscalculated ones in `startTransitionRecursive`
function if there's a FlatList with `removeClippedSubviews` option set.

Fixes software-mansion#2282.

## Changes

- added `Test2282.tsx` repro
- added extra condition in `startTransitionRecursive` function

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- added `Test2282.tsx` repro

## Checklist

- [x] Ensured that CI passes
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.

Incorrect childCount in removeViewAt
4 participants