Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity
23,523 Commits 541 Branches 635 Tags
77ecc7ede1da8fc590d7bc238a2fc02daa736746
Commit Graph

16 Commits

Author SHA1 Message Date
Ramanpreet Nara ddc4225dd4 Move NativeModule initialization logic outside of setModuleRegistry 
Summary:
## Context
A while ago, we introduced a new initialization API in NativeModules via RCTInitializing.h (diff: D28435078 (https://github.com/facebook/react-native/commit/9b45df1fced066f40034b0a58be6f4caafd5f785)).

## Problem
A number of our NativeModules still use setModuleRegistry to perform initialization.

## Changes
This diff migrates those NativeModules to instead use the initialize API.

## Motivation
In bridgeless mode each NativeModule object is [created and decorated by the RCTInstance](https://www.internalfb.com/code/fbsource/[89f6c0df78e453a20555975e06bc46b4e0d2bbe9]/fbobjc/Apps/Internal/Venice/Core/RCTInstance.mm?lines=180-189), while [holding the TurboModuleManagerDelegate mutex](https://www.internalfb.com/code/fbsource/[c50ce2bb3fb078d28e1f9afdef5e8793f1413472]/xplat/js/react-native-github/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm?lines=429%2C431). After D30753286, setModuleRegistry will be called in RCTInstance getModuleInstanceForClass, which means that we'll start calling setModuleRegistry while holding the TurboModuleManagerDelegate lock. This leads to a deadlock, because calling setModuleRegistry on RCTDeviceInfo [creates RCTAccessibilityManager](https://www.internalfb.com/code/fbsource/[89f6c0df78e453a20555975e06bc46b4e0d2bbe9]/xplat/js/react-native-github/React/CoreModules/RCTDeviceInfo.mm?lines=50), which tries to acquire the TurboModuleManagerDelegate lock again. The NativeModule initialize method isn't called while holding the TurboModuleManagerDelegate lock. That's why moving all initialization logic to the initialize method mitigates this deadlock hazard.

In general, we shouldn't do any sort of initialization inside setters for these bridge/bridgeless APIs. No other NativeModules do initialization outside of initialize.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D30754870

fbshipit-source-id: 7c2d50f995cba6f58ee2dfebfabd36f640579bcb
 2021-09-08 12:46:35 -07:00
Samuel Susla beb3c2b538 Back out "Flush operations queue when animation starts in RCTNativeAnimatedModule" 
Summary:
Changelog: [internal]

Original commit changeset: d3fc021dd434

Reviewed By: motiz88

Differential Revision: D30223203

fbshipit-source-id: 8edf79e109858855d13a36fabab2bcae36180df2
 2021-08-10 09:48:38 -07:00
Samuel Susla f3374d0508 Flush operations queue when animation starts in RCTNativeAnimatedModule 
Summary:
changelog: [internal]

If nodesManager has the information if animated node is managed by Fabric, we can't decide if the operation queue should be flushed before it is flushed. Therefore, keep the information about animated nodes inside a set instead of nodesManager.

For simplicity, I will refer to class `RCTNativeAnimatedTurboModule` as *NativeAnimated* and to `RCTNativeAnimatedNodesManager` as *NodesManager*

Notice that each call to *NativeAnimated* is queued up in `_operations` or `_preOperations`. When the queues are flushed, only then methods are called on `RCTNativeAnimatedNodesManager`.

There are two mechanisms that flush operations.
One is triggered by `RCTMountingManager` before mounting operations are applied and after they are applied. This works fine but is important to paint the picture.

The second mechanism is inside `[RCTNativeAnimatedTurboModule startAnimatingNode]`. It flushes the queues for Fabric nodes only (not sure why Fabric nodes only, I couldn't find any explanation in old diffs). It checks with *NativeAnimated* if a node is managed by Fabric. Keep in mind, *NodesManager* only knows about the nodes when the queues have been flushed.

Exampe:
JavaScript calls methods on *NativeAnimated*.
For example:
1. `createNode`
2. `connectAnimatedNodeToView`
3. `startAnimatingNode`. (here, the queues should be flushed, since we are in Fabric)

All of these operations are queued up and for as long as `RCTMountingManager` executes mounting, all proceeds as expected.
But if those operations happen after mounting phase, `startAnimatingNode` will not flush the operations queues, because it can't tell if nodeTag is managed by fabric or it isn't. This is because *NodesManager* hasn't been notified about any new nodes.

Reviewed By: RSNara

Differential Revision: D30099010

fbshipit-source-id: d3fc021dd4346d1cbbda3b49ecd9d982c543e705
 2021-08-09 12:20:28 -07:00
Ramanpreet Nara 1f883192ec Manual: Migrate from bridge.eventDispatcher to RCTModuleRegistry 
Summary:
This is an extension of D25449795. I searched for all usages of .eventDispatcher within NativeModules, and migrated them all to the Venice-compatible RCTModuleRegistry API.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D25473844

fbshipit-source-id: 2b8deec236e019f3adfb59fadd745c249ff822f4
 2020-12-11 10:42:28 -08:00
Scott Kyle 23717e48af Call stopObserving on correct queue 
Summary:
Since `dealloc` can be called from any thread, this would result `stopObserving` being called on a different thread/queue than the specified `methodQueue`. We specifically encountered this issue with a module needing the main queue having its `stopObserving` called on a background queue.

Changelog:
[iOS][Fixed] - Call [RCTEventEmitter stopObserving] on specified method queue

Reviewed By: RSNara

Differential Revision: D23821741

fbshipit-source-id: 693c3be6876f863da6dd214a829af2cc13a09c3f
 2020-09-21 17:30:34 -07:00
Peter Argany 75197dbb16 Pass RCTSurfacePresenter into RCTNativeAnimatedNodesManager in bridgeless 
Summary:
This diff ties the stack together. It completes the long chain of RCTSurfacePresenter ownership:

`FBReactModule` -> `RCTNativeAnimatedTurboModule` (this diff and D23272746)
`RCTNativeAnimatedTurboModule` -> `RCTNativeAnimatedNodesManager` (this diff)
`RCTNativeAnimatedNodesManager` -> `RCTPropsAnimatedNode` (D23272735)

It completes animations working without the bridge.

Changelog: [Internal]

Differential Revision: D23272755

fbshipit-source-id: 137f7ff89993a2cb644bd67869eb685afcec4068
 2020-08-24 16:37:50 -07:00
Peter Argany 700960c9f1 Pass SurfacePresenter directly instead of relying on bridge 
Summary:
`RCTPropsAnimatedNode` uses the bridge to access it's current surface presenter to perform animations.

In bridgeless mode, the surface presenter is not owned by the bridge. Instead, pass the surface presenter through the ownership chain:
`RCTNativeAnimated*Module` -> `RCTNativeAnimatedNodesManager` -> `RCTPropsAnimatedNode`

`RCTSurfacePresenter` should not be strongly held by any of these animation internals. If it gets destroyed at a higher level, animations should not be completed.

Changelog: [Internal]

Differential Revision: D23272735

fbshipit-source-id: ce08ee3b59ac2ba70e31cebb7ba8e9f3a644c848
 2020-08-24 16:37:49 -07:00
Peter Argany e1d53ce697 Create init method for RCTNativeAnimatedModule and it's TM fork 
Summary:
The animated native module relies on `setBridge` to perform generic setup which doesn't rely on the bridge at all. This diff refactors that setup code to an `init` function.

Changelog: [Internal]

Differential Revision: D23272427

fbshipit-source-id: 0c9c5522c9044283f4db25360010465ff42aba25
 2020-08-24 16:37:49 -07:00
Peter Argany d1c3c2d084 Fork RCTAnimatedModule to unblock brideless mode 
Summary:
The Animated native module cannot be converted to a TM because it has perf regressions when used in conjunction with Paper renderer. Instead of fixing these complicated perf issues (which will disappear when Fabric ships this half), temporarily fork the native module. The new fork is converted to a TM, and only used to unblock bridgeless mode testing.

Changelog:[Internal]

Reviewed By: RSNara

Differential Revision: D22651010

fbshipit-source-id: 912123ef38ac8c66025b7bba34a65ec6d98f330d
 2020-07-27 15:09:33 -07:00
TMaszko d92284216f Fix: Preserve native animated value after animated component unmount (#28841) 
Summary:
After animation has been finished using Native driver there is no final value passed from the native to JS side. This causes a bug from https://github.com/facebook/react-native/issues/28114.

This PR solves this problem in the same way as `react-native-reanimated` library. When detaching it is calling native side to get the last value from Animated node and stores it on the JS side.

Preserving animated value even if animation was using `useNativeDriver: true`
Fixes https://github.com/facebook/react-native/issues/28114

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Internal] [Fixed] - Save native Animated node value on JS side in detach phase
Pull Request resolved: https://github.com/facebook/react-native/pull/28841

Test Plan: Unit tests for added getValue method passed. Green CI

Reviewed By: mdvacca

Differential Revision: D22211499

Pulled By: JoshuaGross

fbshipit-source-id: 9a3a98a9f9a8536fe2c8764f667cdabe1f6ba82a
 2020-06-29 17:09:37 -07:00
Ramanpreet Nara e6d7f4a153 Make RCTNativeAnimatedModule a regular NativeModule 
Summary:
There's a race when RCTNativeAnimatedModule is a TurboModule that breaks animations. More details here: T68603674. Even though this is a bit edge-casey, I think it's best to make this NativeModule a non-TurboModule until this problem is resolved.

Changelog: [iOS][Fixed] - Make RCTNativeAnimatedModule a regular NativeModule to unbreak animations with TurboModules on

Reviewed By: yungsters

Differential Revision: D22149179

fbshipit-source-id: 6bf9f0d5eab7c99a82b21ec07e76359181dcedd2
 2020-06-19 18:24:38 -07:00
Ramanpreet Nara 03bd7d799e Part 2: Update ObjC++ codegen classes to use ObjCTurboModule::InitParams 
Summary:
## Summary
Please check out D21035209.

## Changes
- Codemod all ObjC NativeModule `getTurboModuleWithJsInvoker:nativeInvoker:perfLogger` methods to `getTurboModule:(const ObjCTurboModule::Args)`

## Script
```
var withSpaces = (...args) => args.join('\s*')

var regexString = withSpaces(
  '-',
  '\(',
  'std::shared_ptr',
  '<',
  '(?<turboModuleClass>(facebook::react::|react::|::|)TurboModule)',
  '>',
  '\)',
  'getTurboModuleWithJsInvoker',
  ':',
  '\(',
  'std::shared_ptr',
  '<',
  '(?<fbNamespace>(facebook::react::|react::|::|))CallInvoker',
  '>',
  '\)',
  '(?<jsInvokerInstance>[A-Za-z0-9]+)',
  'nativeInvoker',
  ':',
  '\(',
  'std::shared_ptr',
  '<',
  '(facebook::react::|react::|::|)CallInvoker',
  '>',
  '\)',
  '(?<nativeInvokerInstance>[A-Za-z0-9]+)',
  'perfLogger',
  ':',
  '\(',
  'id',
  '<',
  'RCTTurboModulePerformanceLogger',
  '>',
  '\)',
  '(?<perfLoggerInstance>[A-Za-z0-9]+)',
  '{',
  'return',
  'std::make_shared',
  '<',
  '(?<specName>(facebook::react::|react::|::|)Native[%A-Za-z0-9]+SpecJSI)',
  '>',
  '\(',
  'self',
  ',',
  '\k<jsInvokerInstance>',
  ',',
  '\k<nativeInvokerInstance>',
  ',',
  '\k<perfLoggerInstance>',
  '\)',
  ';',
  '}',
)

var replaceString = `- (std::shared_ptr<$<turboModuleClass>>) getTurboModule:(const $<fbNamespace>ObjCTurboModule::InitParams &)params
{
  return std::make_shared<$<specName>>(params);
}`

const exec = require('../lib/exec');
const abspath = require('../lib/abspath');
const relpath = require('../lib/relpath');
const readFile = (filename) => require('fs').readFileSync(filename, 'utf8');
const writeFile = (filename, content) => require('fs').writeFileSync(filename, content);

function main() {
  const tmFiles = exec('cd ~/fbsource && xbgs -n 10000 -l getTurboModuleWithJsInvoker:').split('\n').filter(Boolean);

  tmFiles
    .filter((filename) => !filename.includes('microsoft-fork-of-react-native'))
    .map(abspath)
    .forEach((filename) => {
      const source = readFile(filename);
      const newSource = source.replace(new RegExp(regexString, 'g'), replaceString);

      if (source == newSource) {
        console.log(relpath(filename));
      }

      writeFile(filename, newSource);
    });
}

if (!module.parent) {
  main();
}
```

## Re-generating diff
```
> hg revert -r .^ --all
> node index.js # run script
```

Changelog: [iOS][Changed] - Make all ObjC NativeModules create TurboModules using ObjCTurboModule::Args

Reviewed By: PeteTheHeat

Differential Revision: D21036265

fbshipit-source-id: 404bcc548d1775ef23d793527606d02fe384a0a2
 2020-04-16 17:29:55 -07:00
Ramanpreet Nara bf2609dca5 Make RCTNativeAnimatedModule into a TurboModule 
Summary:
D20831545 integrated TurboModules with the bridge's `onBatchComplete` event. This fixed the RCTNativeAnimatedModule jank, so I'm re-converting RCTNativeAnimatedModule into a TurboModule.

Changelog:
[iOS][Fixed] - Make RCTNativeAnimatedModule TM-compatible

Reviewed By: PeteTheHeat

Differential Revision: D20850744

fbshipit-source-id: bb85a1bb27963e7d39bf149d0a3d7b71c88175da
 2020-04-08 13:04:10 -07:00
Ramanpreet Nara a95184cbad Make RCTNativeAnimatedModule a regular NativeModule 
Summary: We suspect that RCTNativeAnimatedModule's conversion to TurboModule could be the cause. There could be a memory leak in the TurboModule system that RCTNativeAnimatedModule exposes, or RCTNativeAnimatedModule itself could have a memory leak. Therefore, I'm making RCTNativeAnimatedModule a regular NativeModule for now.

Reviewed By: fkgozali

Differential Revision: D18410852

fbshipit-source-id: dd714fe0fb0267fe5e1a94a26d47a82199b6f2c7
 2019-11-12 13:56:09 -08:00
Janic Duplessis 686ab49107 Don't restore default values when components unmount (#26978) 
Summary:
There are some cases where restoring default values on component unmount is not desirable. For example in react-native-screens we want to keep the native view displayed after react has unmounted them. Restoring default values causes an issue there because it will change props controlled my native animated back to their default value instead of keeping whatever value they had been animated to.

Restoring default values is only needed for updates anyway, where removing a prop controlled by native animated need to be reset to its default value since react no longer tracks its value.

This splits restoring default values and disconnecting from views in 2 separate native methods, this way we can restore default values only on component update and not on unmount. This takes care of being backwards compatible for JS running with the older native code.

## Changelog

[General] [Fixed] - NativeAnimated - Don't restore default values when components unmount
Pull Request resolved: https://github.com/facebook/react-native/pull/26978

Test Plan:
- Tested in an app using react-native-screens to make sure native views that are kept after their underlying component has been unmount don't change. Also tested in RNTester animated example.

- Tested that new JS works with old native code

Reviewed By: mmmulani

Differential Revision: D18197735

Pulled By: JoshuaGross

fbshipit-source-id: 20fa0f31a3edf1bc57ccb03df9d1486aba83edc4
 2019-11-04 15:40:09 -08:00
Ramanpreet Nara 791931454e Make RCTNativeAnimatedModule TurboModule-compatible 
Summary:
**Note:** I had to relax the `AnimatedNodeConfig` and `AnimatingNodeConfig` flow types, because they didn't have all the properties. I created a task to improve the flow types.

Changelog: [iOS][Added] Make RCTNativeAnimatedMoudle TurboModule-compatible

Reviewed By: PeteTheHeat

Differential Revision: D17692715

fbshipit-source-id: ab3680c30ca3d5b1eb866ef1104587cae695ad15
 2019-10-16 19:00:26 -07:00