Skip to content

fix(iOS): look through whole ancestor chain when looking for screenview from content wrapper #2683

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

Merged

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Feb 11, 2025

Description

In #2670 I've added a check that logged RN error in case contentWrapper.reactSuperview is not a RNSScreenView.
Now I got reports (but no reproduction :/) from two independent sources that this error is triggered pretty often,
which is unexpected from my perspective. I blindly blame view flattening / usage of legacy navigators but can not be sure.

This PR is an attempt to patch this behaviour.

Changes

contentWrapper now searches whole view ancestor chain before logging an error.

Test code and steps to reproduce

No reproducer :/

Checklist

@kkafar kkafar merged commit 21aca32 into main Feb 11, 2025
5 checks passed
@kkafar kkafar deleted the @kkafar/handle-unexpected-parent-when-attaching-content-wrapper branch February 11, 2025 15:21
@RohovDmytro
Copy link

It did start to work in most places, so you seem to be right. Cool!

At least one place is broken, when the app gets logged out and I remove native stack container for authorized users with a container for guests.

image

I'll let you know if I can pinpoint any useful details.

@kkafar
Copy link
Member Author

kkafar commented Feb 11, 2025

@RohovDmytro thanks for testing & feedback. The best for me would be to get a reproduction of the issue. Then I'll be able to patch this correctly.

@RohovDmytro
Copy link

@kkafar I've created a bunch of issues that are highlighting my findings of the last week. I hope that's more helpful as they have reproductions. Thanks for looking at them, kkafar! Ping me if you will need any details on any issue.

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.

2 participants