Skip to content

fix: copy state in reanimated commit hook#5934

Merged
piaskowyk merged 2 commits intomainfrom
@wolewicki/copy-state
Apr 25, 2024
Merged

fix: copy state in reanimated commit hook#5934
piaskowyk merged 2 commits intomainfrom
@wolewicki/copy-state

Conversation

@WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Apr 24, 2024

Summary

Fix for the issue: Expensify/App#40048 which root cause of seems to be the not copied state of ScrollView when this code is not added. I'll try to provide a simple repro when I get some time to do it.

Test plan

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR!

@tomekzaw
Copy link
Member

Just wondering if we should also add explicit state here?

auto newChildNode = source->clone({/* .props = */ props});

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

I think we should pass state explicitly there as well

@WoLewicki
Copy link
Member Author

Done

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

Thanks!

@piaskowyk piaskowyk added this pull request to the merge queue Apr 25, 2024
Merged via the queue into main with commit 7c45f81 Apr 25, 2024
@piaskowyk piaskowyk deleted the @wolewicki/copy-state branch April 25, 2024 14:09
piaskowyk pushed a commit that referenced this pull request Apr 26, 2024
## Summary

Fix for the issue: Expensify/App#40048 which
root cause of seems to be the not copied state of `ScrollView` when this
code is not added. I'll try to provide a simple repro when I get some
time to do it.

## Test plan
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.

3 participants