(iOS) - NativeCommands fail in ref functions if batchRenderingUpdatesInEventLoop is active

[RyanCommits]

Description

Problem

Starting in version 0.74.1 and above, due to this change, the batchRenderingUpdatesInEventLoop feature flag is turned ON. This causes NativeCommands called in ref functions to fail.

<RTNCenteredText
  {...props}
  ref={element => {
    if (element) {
      Commands.trigger(element); // <-- trigger will not be called natively if batchRenderingUpdatesInEventLoop is turned ON
    }
  }}
/>

The corresponding NativeCommand:

// RTNCenteredText.mm
- (void)trigger {
    NSLog(@"*** Fabric component trigger method called directly");
}

- (void)handleCommand:(const NSString *)commandName args:(const NSArray *)args {
    NSString *TRIGGER = @"trigger";
    if([commandName isEqual:TRIGGER]) {
        [self trigger];
    }
}

Diagnosis

The NativeCommand trigger fails because when synchronouslyDispatchCommandOnUIThread gets called, findComponentViewWithTag returns nil because the _registry does not contain the element.

When batchRenderingUpdatesInEventLoop is off, the _registry is correctly populated with all elements on the screen, and the NativeCommand trigger functions correctly.

If Commands.trigger is wrapped in a setTimeout, it gets called successfully.

<RTNCenteredText
  {...props}
  ref={element => {
    if (element) {
      setTimeout(() => {
        Commands.trigger(element); // <-- trigger gets called successfully
      }, 0);
    }
  }}
/>

Steps to reproduce

See the reproducer provided.

1. Use codegenNativeComponent and codegenNativeCommands to create a NativeCommand
2. Call the created NativeCommand in a ref function
3. See that the NativeCommand does NOT get called in versions >=0.74.1

React Native Version

0.74.5

Affected Platforms

Runtime - iOS

Areas

JSI - Javascript Interface, Bridgeless - The New Initialization Flow

Output of npx react-native info

N/A

Stacktrace or Logs

N/A

Reproducer

https://github.com/RyanCommits/RN74-issue-reproducer

Screenshots and Videos

No response

[react-native-bot]

:warning:

Newer Version of React Native is Available!

:information_source:

You are on a supported minor version, but it looks like there's a newer patch available - 0.74.5. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

[react-native-bot]

:warning:

Newer Version of React Native is Available!

:information_source:

You are on a supported minor version, but it looks like there's a newer patch available - undefined. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

[RyanCommits]

Upgraded reproducer to 0.74.5

[elencho]

@cortinico can I work on this?

[cortinico]

@elencho feel free to investigate further.

Context from @rubennorte:

Clarification: this problem isn't specific to that flag, but some changes in timing can make it worse. That kind of code isn't guaranteed to work correctly with that flag disabled either.

Context: on Android, view mutations and view commands are dispatched in the same queue, so if you create a view (which happens before you get a ref) and you dispatch a command (which you do through a ref), those operations happen in order. On iOS, view mutations and view commands are processed separately (view mutations are queued but view commands are immediately dispatched to the host platform). This causes the request for the view to be queued and the view command to be dispatched immediately, which causes the command not to find the view in most cases (especially if we delay creation via batchRenderingUpdatesInEventLoop).

Solution: the solution for this is to queue view commands in the same queue where we queue view mutations on iOS.

[RyanCommits]

Our library is dependent on the immediate firing of Commands in the component ref.

As we wait for this change, what would you @cortinico suggest as a workaround for now on this race condition? Is using requestAnimationFrame to wrap Commands a stable enough of a workaround for the view mutations to finish before the view commands fire?

[rubennorte]

Our library is dependent on the immediate firing of Commands in the component ref.

As we wait for this change, what would you @cortinico suggest as a workaround for now on this race condition? Is using requestAnimationFrame to wrap Commands a stable enough of a workaround for the view mutations to finish before the view commands fire?

requestAnimationFrame is essentially setTimeout(callback, 0) at the moment, so it's not really helpful for this.

I'm not familiar with your use case, so it's hard to suggest a workaround. Maybe you can queue those commands in native instead?

[RyanCommits]

At Fullstory we use method swizzling to get React classes to call our Commands.

All of this is to allow us to read a custom prop in the native code like: <View customProp="customValue">

In new architecture, everything is strongly typed in C++, so this is our workaround to read customProp values off of a component for our native code to process.

We need to read these props on view creation, and since it's React Classes that we're calling Commands on, I think to leverage a queue, we'd have to rely on React Native's internal implementation.

[rubennorte]

@RyanCommits oh, I see. Would it make sense to use custom components instead of the RN built-ins in that case? Maybe replace View with a custom implementation that handles those attributes properly?

[RyanCommits]

The idea is for our users to be able to plug and play - install our plugin with minimal code changes and be able to tag their components. We could build a custom component, but then our users will have to replace every component that requires this feature. We would also lose the ability to use our babel plugin to auto-tag components with identifying props, including components within other libraries.

[rubennorte]

We'll track the fix for this in #47576.