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

fix: Allow viewRegistry to accept subclasses of NSView #2196

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Sep 19, 2024

Summary:

In React Native macOS, we have two "shim" classes to help make the iOS codebase of React Native compatible with macOS

  • RCTPlatformView:a typedef of NSView on macOS, and UIView on iOS / visionOS)
  • RCTUIView: A subclass of NSView with extra methods and properties to be closer to the iOS UIView

Additionally, in React Native's old architecture, there is this line that we forked as follows in 0.73 and earlier versions:

React Native:

typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry);

React Native macOS:

typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, NSDictionary<NSNumber ,
RCTUIView
> *viewRegistry); // [macOS]`

In React Native macOS 0.74+, I changed the definition as follows, thinking the widening of scope to a superclass would be a non breaking change:

typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, NSDictionary<NSNumber ,
RCTPlatformView
> *viewRegistry); // [macOS]`

Turns out I was wrong, and this broke libraries like Reanimated and React Native Gesture Handler. The fix is simple, add a __kindof in front of the type!

typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, NSDictionary<NSNumber ,
__kindof RCTPlatformView
> *viewRegistry); // [macOS]`

Test Plan:

Tested this in my branch where I'm adding macOS support to react-native-webgpu (wcandillon/react-native-webgpu#123), and reanimated + gesture gesture handler compile successfully.

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