fix(ios): make the unmountChildComponentView assert message bounds-safe - #57865
fix(ios): make the unmountChildComponentView assert message bounds-safe#57865dongdongbh wants to merge 1 commit into
Conversation
|
Hi @dongdongbh! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign 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 to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary
-[RCTViewComponentView unmountChildComponentView:index:]bounds-checks theRCTAssertcondition, but the failure message it formats afterwards calls-objectAtIndex:on the same out-of-rangeindex:So the exact situation the assert exists to report — a shadow-tree/native child-index mismatch — raises
NSRangeExceptionwhile being reported, instead of being reported.That is normally invisible, because a source Release build strips
RCTAssertand the mismatch is harmless:indexis used only inside the asserts, and the real work,[childComponentView removeFromSuperview], never needed it. But the prebuiltReact.frameworkpublished to Maven Central is built with assertions enabled (#57454), so this is live in App Store builds — we hit it on RN 0.81.5 via Expo SDK 54, symbolicated against the publishedreactnative-core-dSYM-releaseartifact:This is the second of the two fixes suggested in #57454 and stands on its own for any build with assertions enabled.
The change also reads
self.currentContainerViewonce instead of four times. That getter is not a plain accessor — it creates or tears down_containerViewand reparents subviews — so re-invoking it inside an assert's arguments is worth avoiding regardless.Changelog:
[IOS] [FIXED] - Report a
RCTViewComponentViewchild-index mismatch instead of raisingNSRangeExceptionwhile formatting the assert messageTest Plan
I don't have a macOS build environment, so I have not compiled this — flagging that plainly. What backs the change:
stringsonreact-native-artifacts-0.81.5-reactnative-core-release.tar.gzshows bothAttempt to unmount…format strings present, confirming the call sites are compiled into the release artifact — same check [iOS] Prebuilt React-Core Release artifacts are built with NSAssertions enabled — RCTAssert failures become fatal NSRangeException crashes in production #57454 reports for 0.85.3 and 0.86.0, so the artifact defect reaches at least as far back as 0.81.out of boundsin place of a tag that cannot be read.Happy to rework this if you would rather the assert drop the
tag at indexfield entirely, or if fixing the artifact build flags is considered sufficient on its own.