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

[Android] [Fixed] Fix the IllegalViewOperationException when updateLayout in NativeViewHierarchyManager #22508

Closed
wants to merge 1 commit into from

Conversation

raphael-liu
Copy link

@raphael-liu raphael-liu commented Dec 4, 2018

[Android] [Fixed] Fix the IllegalViewOperationException when updateLayout in NativeViewHierarchyManager.

TestPlan

No plan.

ChangeLog

Remove the code of throwing IllegalViewOperationException, print the log and return instead.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@pull-bot
Copy link

pull-bot commented Dec 4, 2018

Warnings
⚠️

📋 Test Plan - This PR appears to be missing a Test Plan.

⚠️

📋 Changelog - This PR appears to be missing Changelog.

Generated by 🚫 dangerJS

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 5, 2018
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@cpojer
Copy link
Contributor

cpojer commented Dec 7, 2018

This doesn't really seem like a fix to me as you are just commenting out an exception that is thrown for probably a good reason. I'm gonna close this pull request proactively but I'm happy to reopen it if you update your code, summary or test plan to verify that this change is actually something we want to merge. Thank you.

@cpojer cpojer closed this Dec 7, 2018
@raphael-liu
Copy link
Author

This doesn't really seem like a fix to me as you are just commenting out an exception that is thrown for probably a good reason. I'm gonna close this pull request proactively but I'm happy to reopen it if you update your code, summary or test plan to verify that this change is actually something we want to merge. Thank you.

But at least, the exception message used a wrong tag.

        ViewManager parentViewManager = mTagsToViewManagers.get(parentTag);
        ViewGroupManager parentViewGroupManager;
        if (parentViewManager instanceof ViewGroupManager) {
          parentViewGroupManager = (ViewGroupManager) parentViewManager;
        } else {
          throw new IllegalViewOperationException(
              "Trying to use view with tag " + tag + " as a parent, but its Manager doesn't extends ViewGroupManager");
        }

The tag of the exception message should be the parentTag.

facebook-github-bot pushed a commit that referenced this pull request Jan 2, 2019
Summary: It was pointed out to me in #22508 by raphael-liu that the error message refers to the wrong tag. I didn't merge the PR because I don't have good insight into the effects it could cause, but we should at least fix the error message.

Reviewed By: TheSavior

Differential Revision: D13564266

fbshipit-source-id: fa76f0888364df329d052dbcc2050f906d39dcef
KusStar pushed a commit to KusStar/react-native that referenced this pull request Sep 19, 2023
Summary: It was pointed out to me in facebook#22508 by raphael-liu that the error message refers to the wrong tag. I didn't merge the PR because I don't have good insight into the effects it could cause, but we should at least fix the error message.

Reviewed By: TheSavior

Differential Revision: D13564266

fbshipit-source-id: fa76f0888364df329d052dbcc2050f906d39dcef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: View Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants