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

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

Open
RyanCommits opened this issue Sep 4, 2024 · 5 comments
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Newer Patch Available Platform: iOS iOS applications. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@RyanCommits
Copy link

RyanCommits commented Sep 4, 2024

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

@RyanCommits RyanCommits added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Sep 4, 2024
@react-native-bot
Copy link
Collaborator

⚠️ Newer Version of React Native is Available!
ℹ️ 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 react-native-bot added the Platform: iOS iOS applications. label Sep 4, 2024
@react-native-bot
Copy link
Collaborator

⚠️ Newer Version of React Native is Available!
ℹ️ 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
Copy link
Author

Upgraded reproducer to 0.74.5

@cortinico cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Triage 🔍 labels Sep 12, 2024
@elencho
Copy link
Contributor

elencho commented Sep 17, 2024

@cortinico can I work on this?

@cortinico
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Newer Patch Available Platform: iOS iOS applications. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

4 participants