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

Timing of native view commands is incorrect on iOS in the new architecture #47576

Closed
rubennorte opened this issue Nov 12, 2024 · 3 comments
Closed
Labels
p: Facebook Partner: Facebook Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Tech: Fabric Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@rubennorte
Copy link
Contributor

rubennorte commented Nov 12, 2024

Description

In the new architecture, native view commands are dispatched immediately to the UI thread:

- (void)dispatchCommand:(ReactTag)reactTag commandName:(NSString *)commandName args:(NSArray *)args
{
if (RCTIsMainQueue()) {
// Already on the proper thread, so:
// * No need to do a thread jump;
// * No need to allocate a block.
[self synchronouslyDispatchCommandOnUIThread:reactTag commandName:commandName args:args];
return;
}
RCTExecuteOnMainQueue(^{
[self synchronouslyDispatchCommandOnUIThread:reactTag commandName:commandName args:args];
});
}

But mounting operations are only flushed at the end of the current task in the event loop / runtime scheduler.

That causes problems when invoking native view commands on views that were just created, e.g.:

return (
  <TextInput ref={node => {
    if (node) {
      node.focus();
    }
  }} />
);

In this case, the steps RN follows are:

  1. JS: Commit a new version of the shadow tree.
  2. JS: Mount refs in React.
  3. JS: Dispatch native view command to focus
  4. In parallel:
    4.1 UI: Execute native view command
    4.1 JS: finish task and dispatch mount operations to UI thread.

On Android, this works correctly because we queue the native view command in the same queue as mount operations, so they're always processed in the right order. We need to do something similar on iOS to fix this issue.

React Native Version

0.76

Affected Platforms

Runtime - iOS

Areas

Fabric - The New Renderer

@rubennorte rubennorte added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Nov 12, 2024
@react-native-bot react-native-bot added Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Needs: Version Info labels Nov 12, 2024
@react-native-bot
Copy link
Collaborator

Warning

Could not parse version: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.76.2.

@react-native-bot react-native-bot added Platform: iOS iOS applications. and removed Needs: Triage 🔍 labels Nov 12, 2024
@react-native-bot
Copy link
Collaborator

Warning

Missing reproducer: We could not detect a reproducible example in your issue report. Please provide either:

@react-native-bot
Copy link
Collaborator

Warning

Could not parse version: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.76.2.

@cipolleschi cipolleschi added the p: Facebook Partner: Facebook label Nov 13, 2024
sammy-SC added a commit to sammy-SC/react-native that referenced this issue Nov 14, 2024
Summary:
## Changelog:

[iOS] [Fixed] - Fixed use of view commands from layout effects

Mounting of views is delayed by runtime scheduler to allow React to run layout effects. Execution of view commands must by queued together with mounting of views, otherwise it might be executed before views are mounted. When this happens, view commands are ignored.

So before, if view command was executed from layout effect (or ref function), it would get dispatched to the UI thread as quickly as possible. But mounting of views would be delayed. To fix this, both mounting of views and view commands are put on the same queue inside of RuntimeScheduler.

## What about Android?
Android employs a [retry mechanism](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java#L211) that was needed for react tag based view commands. In paper, one could send a view command to a react tag which was completely disconnected from whether a view exists or not.

iOS was built with ref commands in mind, so it doesn't have this mechanism.


Fixes: facebook#47576

Differential Revision: D65909191
sammy-SC added a commit to sammy-SC/react-native that referenced this issue Nov 14, 2024
…cebook#47604)

Summary:

## Changelog:

[iOS] [Fixed] - Fixed use of view commands from layout effects

Mounting of views is delayed by runtime scheduler to allow React to run layout effects. Execution of view commands must by queued together with mounting of views, otherwise it might be executed before views are mounted. When this happens, view commands are ignored.

So before, if view command was executed from layout effect (or ref function), it would get dispatched to the UI thread as quickly as possible. But mounting of views would be delayed. To fix this, both mounting of views and view commands are put on the same queue inside of RuntimeScheduler.

## What about Android?
Android employs a [retry mechanism](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java#L211) that was needed for react tag based view commands. In paper, one could send a view command to a react tag which was completely disconnected from whether a view exists or not.

iOS was built with ref commands in mind, so it doesn't have this mechanism.


Fixes: facebook#47576

Reviewed By: javache, cipolleschi

Differential Revision: D65909191
@migueldaipre migueldaipre added the Resolution: PR Submitted A pull request with a fix has been provided. label Nov 15, 2024
sammy-SC added a commit to sammy-SC/react-native that referenced this issue Nov 15, 2024
…cebook#47604)

Summary:

## Changelog:

[iOS] [Fixed] - Fixed use of view commands from layout effects

Mounting of views is delayed by runtime scheduler to allow React to run layout effects. Execution of view commands must by queued together with mounting of views, otherwise it might be executed before views are mounted. When this happens, view commands are ignored.

So before, if view command was executed from layout effect (or ref function), it would get dispatched to the UI thread as quickly as possible. But mounting of views would be delayed. To fix this, both mounting of views and view commands are put on the same queue inside of RuntimeScheduler.

## What about Android?
Android employs a [retry mechanism](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java#L211) that was needed for react tag based view commands. In paper, one could send a view command to a react tag which was completely disconnected from whether a view exists or not.

iOS was built with ref commands in mind, so it doesn't have this mechanism.


Fixes: facebook#47576

Reviewed By: javache, cipolleschi

Differential Revision: D65909191
sammy-SC added a commit to sammy-SC/react-native that referenced this issue Nov 15, 2024
…cebook#47604)

Summary:

## Changelog:

[iOS] [Fixed] - Fixed use of view commands from layout effects

Mounting of views is delayed by runtime scheduler to allow React to run layout effects. Execution of view commands must by queued together with mounting of views, otherwise it might be executed before views are mounted. When this happens, view commands are ignored.

So before, if view command was executed from layout effect (or ref function), it would get dispatched to the UI thread as quickly as possible. But mounting of views would be delayed. To fix this, both mounting of views and view commands are put on the same queue inside of RuntimeScheduler.

## What about Android?
Android employs a [retry mechanism](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java#L211) that was needed for react tag based view commands. In paper, one could send a view command to a react tag which was completely disconnected from whether a view exists or not.

iOS was built with ref commands in mind, so it doesn't have this mechanism.


Fixes: facebook#47576

Reviewed By: javache, cipolleschi

Differential Revision: D65909191
sammy-SC added a commit to sammy-SC/react-native that referenced this issue Nov 16, 2024
…cebook#47604)

Summary:

## Changelog:

[iOS] [Fixed] - Fixed use of view commands from layout effects

Mounting of views is delayed by runtime scheduler to allow React to run layout effects. Execution of view commands must by queued together with mounting of views, otherwise it might be executed before views are mounted. When this happens, view commands are ignored.

So before, if view command was executed from layout effect (or ref function), it would get dispatched to the UI thread as quickly as possible. But mounting of views would be delayed. To fix this, both mounting of views and view commands are put on the same queue inside of RuntimeScheduler.

## What about Android?
Android employs a [retry mechanism](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java#L211) that was needed for react tag based view commands. In paper, one could send a view command to a react tag which was completely disconnected from whether a view exists or not.

iOS was built with ref commands in mind, so it doesn't have this mechanism.


Fixes: facebook#47576

Reviewed By: javache, cipolleschi

Differential Revision: D65909191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: Facebook Partner: Facebook Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Tech: Fabric Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants