fix(iOS): forward-declare RCTSurfaceTouchHandler in UIView+RNSUtility.h#3986
Conversation
The public header `ios/utils/UIView+RNSUtility.h` directly imports
`<React/RCTSurfaceTouchHandler.h>`, which is shipped in the
`React-RCTFabric` clang module rather than `React`. Under
`use_frameworks!:linkage => :static` + new architecture, any consumer
module that imports the `RNScreens` clang module (e.g. ExpoRouter)
inherits this transitive import without the matching framework search
path, and Clang's module dependency scanning fails before compilation:
error: Clang dependency scanning failure: While building module
'RNScreens' ... 'React/RCTSurfaceTouchHandler.h' file not found
error: Compilation search paths unable to resolve module dependency:
'RNScreens' (in target 'ExpoRouter' from project 'Pods')
Move the import out of the public surface: forward-declare
`RCTSurfaceTouchHandler` in the header and `#import` it in the .mm.
This matches the convention already used by `RNSScreenStack.mm` and
sibling utility headers (`UINavigationBar+RNSUtility.h`), and keeps
the public module interface free of fabric-internal dependencies.
All existing internal consumers (`RNSScreenStack.mm`, `RNSScreen.mm`,
`UIView+RNSUtility.mm`) already import `RCTSurfaceTouchHandler.h`
directly, so no call sites need updating.
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
Hey, thanks for the PR. Seems like something I'll land. I just have one question.
|
|
||
| #import <Foundation/Foundation.h> | ||
| #import <React/RCTSurfaceTouchHandler.h> | ||
| #import <UIKit/UIKit.h> |
There was a problem hiding this comment.
Why change Foundation import for UIKit import? How is it related to the issue?
There was a problem hiding this comment.
I think it's explained in #3985 ("Suggested fix" section).
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
I'm good with this change. @kligarski would you mind giving a round of test (build with the configuration the OP suggests) & in case of success landing it?
Okay, I'll test it on Monday. |
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
I gave it a round of testing & seems to work fine. Thank you.
I'll cherry-pick this change to 4.25.0 release.
3 participants
Summary
ios/utils/UIView+RNSUtility.hdeclares methods returningRCTSurfaceTouchHandler *and currently#imports<React/RCTSurfaceTouchHandler.h>directly in the public header. That header is shipped in theReact-RCTFabricclang module, notReact— so underuse_frameworks!:linkage => :static+ new arch, any consumer module that imports theRNScreensclang module (e.g. ExpoRouter) fails Clang's module dependency scanning before compilation:This PR moves the import out of the public surface:
UIView+RNSUtility.h: replace#import <React/RCTSurfaceTouchHandler.h>with@class RCTSurfaceTouchHandler;forward declaration. Replace<Foundation/Foundation.h>with<UIKit/UIKit.h>(since the fabric header was previously providing UIKit transitively, and this is aUIViewcategory — matching the siblingUINavigationBar+RNSUtility.h).UIView+RNSUtility.mm: add the real#import <React/RCTSurfaceTouchHandler.h>.This matches the pattern already used by
RNSScreenStack.mm, where fabric-internal imports stay inside the implementation file.Fixes #3985.
Compatibility
All current internal consumers of
UIView+RNSUtility.h(RNSScreenStack.mm,RNSScreen.mm, andUIView+RNSUtility.mmitself) already#import <React/RCTSurfaceTouchHandler.h>directly, so they retain access to the type without relying on transitive imports from the public header. No call sites need updating.The change is purely a compile-time refactor — no symbols, return types, ABI, or runtime behavior change.
Test plan
use_frameworks!:static+ new arch + RN 0.85.3, the patched header allows ExpoRouter to compile against RNScreens. Without the fix,expo run:iosfails on the Clang module dependency scanning error above.use_frameworks!and non-frameworks builds across new/legacy arch.Related
RectUtil.himport path), which fixed a similar consumer-side compilation failure by relocating a header import.useFrameworks: 'static'due to import of missing headers from RNScreens expo/expo#39080 (consumer → RNScreens internal headers), but in the opposite direction (RNScreens → React fabric).