Repository navigation
fix(ios): stop leaking hidden views, Old Arch color crash and misplaced shapes - #22
Merged
Merged
Conversation
…ed shapes - Restore a child's isHidden when Fabric unmounts it during loading, so the recycled view does not come back hidden on another screen. - Let the Old Arch view config process gradientColors (UIColorArray) instead of a JS gate on global._IS_FABRIC, which React Native never defines, and fall back to the default colors when fewer than two arrive. - Convert nested view frames into skeleton coordinates when building the mask. - Declare codegenConfig.ios.componentProvider so the Fabric component is registered explicitly instead of through the deprecated .mm crawl. - Add jest tests for the JS layer and the native contracts, an XCTest package for the Swift core (yarn test:ios) and an XCTest target hosted in the example app for the component and the Old Arch manager (yarn test:ios:example); run both native suites in a new CI job. - Import UIKit explicitly so the Swift core builds standalone. Fixes: #16, #19, #20 Refs: #18 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes four iOS issues. Each one was reproduced before the fix and verified after it; the table says how.
isLoadingis still hidden (isHidden = true). Fabric puts it in the recycle pool, andupdateLayoutMetricsonly resetshiddenwhendisplayTypechanges, so the next owner gets a hidden view.AutoSkeletonViewoverridesunmountChildComponentView:index:and restores the child before it leaves (SkeletonCore.restoreOriginalView).Texts rendered elsewhere. Before: the first one is invisible. After: all three visible. Re-ran with the override disabled to confirm the repro catches it. Also checked unmounting the wholeAutoSkeletonViewwhile loading: the views rendered afterwards are all visible. Unit:SkeletonCoreTests.-[__NSArray0 objectAtIndex:]: index 0 beyond boundson Old ArchprocessColorran only whenglobal._IS_FABRIC === false, a global React Native does not define. The manager declared the prop asNSArray<UIColor>, which the Old Arch view config does not process, so raw strings reachedRCTConvert UIColor, becamenil, were dropped, andAnimationGradientindexed an empty array.UIColorArray, so the view config runsprocessColorArray; the JS gate is removed so colors are never processed twice;AnimationGradientfalls back to the defaults when fewer than two colors arrive.AnimationGradientTestsfails on the old code with the exact exception text from the issue, passes after. jest:gradientColorsis never pre-processed in JS. The Old Arch view config change is proven by reading RN (getProcessorForType), not on a device: the Old Arch example renders nothing at all in this environment (Xcode 26.5 / iOS 26.5), even a screen with a singleText.TouchableOpacitySkeletonViewOldArchcollects nested descendants, but the mask usedoriginalView.frame, which is relative to the view's own parent.TouchableOpacitycreates a stacking context, so its children really are nested.convert(bounds, to:)into skeleton coordinates.AutoSkeletonViewforced through the legacy interop (the path that produces this symptom) by temporarily renamingAutoSkeletonViewClsso the codegen crawl misses it. Before: a shape drawn above the card at the text's local offset. After applying only this change: gone. Unit:SkeletonPlaceholderMaskTests.ios.componentProvider, so registration relied on the deprecated.mmcrawl; when it misses, Fabric falls back to the interop path from #16.codegenConfig.ios.componentProvider.RCTThirdPartyComponentsProvider.mmlists both components.Tests
Every invariant the fixes rely on has a test, and every test that guards a fix was run red by reverting just that fix.
AutoSkeletonViewTests.testUnmountingAChildWhileLoadingRestoresItsVisibility(hosted)unmountChildComponentViewoverridetestUnmountingAnOriginallyHiddenChildKeepsItHidden(hosted)testLeavingTheWindowWhileLoadingRestoresChildren(hosted)restoreOriginalViewhands a view back unhidden, keeps original state, stops managing it, ignores foreign viewsSkeletonCoreTests(4 cases, SwiftPM)SkeletonCoreTests(3 cases, SwiftPM)gradientColorsasUIColorArraytestOldArchGradientColorsPropIsExportedAsUIColorArray(hosted)getNativeComponentAttributesconvert the colors (andshimmerBackgroundColor)nativeContract.test.ts› Old Architecture view config (jest)testOldArchSetterSurvivesUnprocessedColors(hosted)__NSArray0exception from #20testOldArchSetterAppliesProcessedColors(hosted)AnimationGradientTests(SwiftPM)SkeletonPlaceholderMaskTests(SwiftPM)gradientColors; defaults and pass-throughindex.test.tsx(jest)_IS_FABRICgatecomponentProviderlists every codegen component and maps it to a Fabric component classnativeContract.test.ts› Fabric component registration (jest)How they run:
yarn test— jest (11).yarn test:ios—ios-tests/Swift package overios/SkeletonView(12), no pods needed.yarn test:ios:example—AutoSkeletonExampleTests, an XCTest target hosted in the example app, so the ObjC++ component and the Old Arch manager run against the real React and codegen pods (7). Classes are resolved at runtime because the library headers are not public in the podspec.IOS_DESTINATIONpicks the simulator,RCT_METRO_PORTthe Metro port.test-iosjob runs both native suites. It has not been run on GitHub yet; it builds the example on the runner's Xcode, so it can hit the samefmtconsteval error the example has with Xcode 26.Android is unchanged, so no Android tests were added.
Other changes
import UIKitadded to the Swift files that used UIKit types only through the pod's umbrella header, so the core compiles on its own for the tests.AutoSkeletonExampleTeststarget, its scheme entry, andinherit! :search_pathsin the Podfile (headers only; linking the pods again would duplicate every class already in the host app).Podfile.lockonly changes itsPODFILE CHECKSUM.yarn testcurrently fails with "No tests found"; it now has tests.yarn.lockresynced: it still listed@expo/config-pluginsas a dev dependency after it moved to peers, soyarn install --immutablewas already failing.Known limitation
Under the legacy interop path, the
unmountChildComponentViewoverride does not run, so the #19 leak can still happen there. ExplicitcomponentProviderregistration makes that path unlikely.Verification
yarn test11/11,yarn test:ios12/12,yarn test:ios:example7/7yarn typecheck,yarn lintclean (two pre-existing warnings inexample/),yarn install --immutable,yarn prepare, commitlint🤖 Generated with Claude Code