Summary:
This PR is a follow up for the contributing content move on the website:
* https://github.com/facebook/react-native-website/pull/3120
It replaces most of the CONTRIBUTING file content with a reference to the contributing overview page on the website, which has been based off the content of this file.
Additionally I have searched thought the code for the wiki links and replaces theme with the correct website links. There was an instance where comment was referring to an old and removed a while ago wiki page, so I just get rid of this link.
## Changelog
[Internal] [Chore] - Update CONTRIBUTING.md, replace wiki links
Pull Request resolved: https://github.com/facebook/react-native/pull/34035
Test Plan: N/A
Reviewed By: lunaleaps
Differential Revision: D37318814
Pulled By: cortinico
fbshipit-source-id: d3f5e5c5bd477c0de5c4f0f1d5de81f464b9f5b4
Summary:
We currently wrap colors in an object to make it look similar to a `PlatformColor` object, but since this is a hot codepath, let's just optimize it to a simple array of strings. The next step is to apply a layer of caching here, but this should be a simple improvement.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D31057046
fbshipit-source-id: f68e17167ddd5bba3b545d039600c7c9b40808f5
Summary:
I have never seen these asserts fire in production. They're pretty cheap but the cost is not zero. We will use annotations and test carefully in debug if we need to ensure that something runs on a particular thread - which we do anyway.
Motivation: this method is called /extremely frequently/, everywhere in the mounting layer. And 99.99% of the time it's completely useless and results in absolutely no signal. In many cases, it will be called hundreds or thousands of times during a single operation (for example, when executing the IntMountBuffer items, each sub-item will call this many times).
Wall-clock time is usually low according to systrace (sometimes there are odd spikes), but over time these do add up and it seems good to save a few ms here and there.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D36539399
fbshipit-source-id: 3e023be64b8c9f0e6c3c8347c077ce9fa38f74a4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33776
The implementations of these modules is pretty much identical, and we're already shipping the fbjni version of this anyway.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D36200330
fbshipit-source-id: 135ee621e1e4c5eb9616ce7f442fc6d4b946f865
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33780
Currently this just has a default ThreadPool name, which can be confusing in systraces and crash reports.
Changelog: [Internal]
Reviewed By: ryancat
Differential Revision: D36200090
fbshipit-source-id: 22918993e7c822ed721ccaf79cdcd9d2a972193d
Summary:
Assertion failure would lead to a crash, in real world not being able to start an activity shouldn't crash - usually a navigation won't happen as expected, user could try again.
Changelog:
[Android][Changed] - Don't assert on current activity when call startActivityForResult
Reviewed By: cortinico
Differential Revision: D35746652
fbshipit-source-id: 0b77ca5a69b2f3f3b0b969d84980ed8290ac9b1f
Summary:
Now that the PFH node has been renamed this updates the pfh label.
Produced via `xbgs -l -e '"pfh:ReactNative_CommonInfrastructurePlaceholde"' | xargs sed -i 's/"pfh:ReactNative_CommonInfrastructurePlaceholde"/"pfh:ReactNative_CommonInfrastructurePlaceholder"/'`
Reviewed By: jkeljo
Differential Revision: D35374087
fbshipit-source-id: 61590f69de5a69ec3b8a0478f6dd43409de3c70b
Summary:
## Rationale
The CatalystInstance exposes a public API that returns a list of all instantiated NativeModules: [CatalystInstance.getNativeModules()](https://www.internalfb.com/code/fbsource/[d9cd2e5dd41fc3e3022bfb777c8b31c92af8c537]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java?lines=73).
This enables a use-case: process all NativeModules that conform to a particular interface, to, for example, remove sensitive data before logging out.
## Changes
This diff moves that CatalystInstance.getNativeModules() API into the ReactContext. This allows us to migrate NativeModules leveraging this use-case off of ReactContext.getCatalystInstance(), which makes them bridgeless-mode compatible.
Changelog: [Android][Added] Introduce ReactContext.getNativeModules()
Reviewed By: sshic
Differential Revision: D35286940
fbshipit-source-id: 1b64351aa3f65ee59e6438a7a95974e219ccc69c
Summary:
While it would be better to be able to do all of the ownership metadata at the Buck macro level, that proved to be more work than expected.
This diff adds the corresponding pfh label to all targets in `xplat/js/react-native-github` that have a Supermodule label. Once the migration is complete the Supermodules labels will be able to be removed.
Reviewed By: cortinico
Differential Revision: D35221544
fbshipit-source-id: d87d5e266dfb5e6ee087251dc34dff5db299bbaf
Summary:
Provides an alternative way of diffing props on Android side. Instead of passing dynamic JSON values from JS, the new approach diff C++ props and serializes difference into a `MapBuffer`. Current implementation does this only for `RCTView` component to test performance, correctness and memory impact (as MapBuffers are supposed to be more compact and performant).
This way of passing props allows for additional alignment between platforms, as C++ props are now source of truth for the view state, similar to iOS. At the same time, changing serialization method requires reimplementing `ViewManager` codegen (done manually for now) to account for `MapBuffer`s.
Changelog: [Added][Android] - Added an experimental prop serialization path based on MapBuffer
Reviewed By: mdvacca
Differential Revision: D33735245
fbshipit-source-id: 1515b5fe92f6557ae55abe215ce48277c83974a6
Summary:
I was seeing a SoftException in our apps due to this method being called as part of ReactInstanceManager's init. It seems inconsistent that we would call this from a background thread when all other `onHost*` methods are marked as confined to the UI thread.
Changelog: [Internal]
Reviewed By: ryancat
Differential Revision: D34340210
fbshipit-source-id: 0104eda66b2ca6bb315e64e806e9a30409d0d45c
Summary:
This diff deletes the flag ReactFeatureFlags.enableReactContextCleanupFix, the flag was disabled for many months, I just disable it.
changelog: [internal] internal
Reviewed By: genkikondo, makovkastar
Differential Revision: D33781628
fbshipit-source-id: 4b5e22adf9d30da5b85bbbde8bdc98d98f5e8891
Summary:
Adds a return value to `MessageQueueThread#runOnQueue`, it's implementors and one caller.
It is currently possible for a callback to not be called (because the HandlerThread has been shutdown). If the callback is mission-critical (frees resources, releases a lock, etc) the callee has no indication that it needs to do something.
This surfaces that information to the callee.
Changelog:
[Android][Changed] - Made `MessageQueueThread#runOnQueue` return a boolean. Made `MessageQueueThreadImpl#runOnQueue` return false when the runnable is not submitted.
Reviewed By: RSNara
Differential Revision: D33075516
fbshipit-source-id: bd214a39ae066c0e7d413f2eaaaa05bd072ead2a
Summary:
Added more logs to understand what's the root cause for https://fburl.com/logview/kgknonri
```java.lang.IllegalStateException: Message queue threads already initialized
at X.5y2.A0I(:64)
at com.facebook.venice.ReactInstance.<init>(:112)
at X.PrB.EgB(:33)
at X.2pN.run(:4)
at X.2pA.execute(:32)
at X.2p6.A00(:30)
at X.2p6.A08(:2)
at X.PrC.EgB(:26)
at X.Pr7.run(:4)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
at java.lang.Thread.run(Thread.java:919)
```
Changelog:
[Android][Changed] - Add some logs
Reviewed By: RSNara
Differential Revision: D31584264
fbshipit-source-id: 11b8bb2c6c9af2266688e3dae95e09f0160de79a
Summary:
This diff is hooking up cxx error with redbox. Before this diff, cxx errors were only shown in log and there was no visible user feedback.
Changelog:
[Android] [Added] - Show Redbox for C++ errors.
Reviewed By: JoshuaGross
Differential Revision: D30421355
fbshipit-source-id: ad473337ba301feb08ba31ee8d82ebaa771ecaeb
Summary:
After this diff, when ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse is enabled, the legacy NativeModule infra will log soft exceptions whenever legacy NativeModules are accessed/used.
Changelog: [Internal]
Reviewed By: p-sun
Differential Revision: D30272695
fbshipit-source-id: 7111402c1d8b883a600dcb4559e9ff1d56447070
Summary:
When the TurboModule system is enabled, C++ NativeModules shouldn't be used in production. We'll use this JReactSoftExceptionLogger to log soft exceptions from C++ NativeModules this scenario.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D30272694
fbshipit-source-id: 8dadcfe51bcbc353d438d1a403e74da5e2cb9546
Summary:
When ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse is true, we will log a SoftException, whenever:
1. A Java/Cxx NativeModule is created by the legacy system.
2. Any method on the Java NativeModule is executed, including getConstants().
NOTE: Logs to CXXModule use incoming.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D30252953
fbshipit-source-id: 570929624d0114bb298c593ba909e5cdbd54bd6c
Summary:
ReactSoftException makes it seem like the class is an Exception, when it's actually a logger. This rename I think is more appropriate.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D30251735
fbshipit-source-id: 550bd543388e698774ec31753200bd3f722d8c17
Summary:
After creating and using this utility, we learned that (1) it's really useful, and (2) its interface is good enough. So, let's un-deprecate this utility.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D30251651
fbshipit-source-id: d1ecf81484f865587a5552d5ddf0e68da86397d9
Summary:
**Context:**
Issue:
```java.lang.IllegalStateException: Message queue threads already initialized
at com.facebook.react.bridge.ReactContext.initializeMessageQueueThreads(ReactContext.java:100)
```
Possible root cause:
1, ReactContext.initializeMessageQueueThreads() can be called from different threads, so it's not thread safe
2, ReactContext.initializeMessageQueueThreads() is called again without detroying the threads.
**Fix:**
Make initializeMessageQueueThreads() thread safe. If this won't fix T93983690, I'll continue investigating with possible root cause 2.
Changelog:
[Android][Changed] - Make initializeMessageQueueThreads() thread safe
Reviewed By: GijsWeterings
Differential Revision: D29877386
fbshipit-source-id: 11b32c7184e9e8d6f882474fd607538df12276b7
Summary:
The RuntimeExecutor that Fabric gets from the bridge doesn't call JSIExecutor::flush(). In the legacy NativeModule system, we're supposed to flush the queue of NativeModule calls after every call into JavaScript. The lack of this flushing means that we execute NativeModule calls less frequently with Fabric enabled, and TurboModules disabled. It also means that [the microtask checkpoints we placed inside JSIExecutor::flush()](https://www.internalfb.com/code/fbsource/[62f69606ae81530f7d6f0cba8466ac604934c901]/xplat/js/react-native-github/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp?lines=427%2C445) won't be executed as frequently, with Fabric enabled.
Changelog: [Android][Fixed] - Flush NativeModule calls with Fabric on Android, on every Native -> JS call.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D28620982
fbshipit-source-id: ae4d1c16c62b6d4a5089e63104ad97f4ed44c440
Summary:
This diff creates a ReactFeatureFlag to initialize MapBufferSo file during Fabric initialization.
This is necessary to be able to compare properly Mapbuffer vs ReadableNativeMap (because ReadableNativeMap c++ files is already included in the bridge so file)
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D28436044
fbshipit-source-id: 338e1bb72b5313dc29a309e1b0e979e7c8bd1c18
Summary:
With D27975839, RuntimeExecutor will be able to start flushing the queued up NativeModule calls in every call from C++ -> JavaScript. We're going to test this feature to measure its impact. In this diff, I wire up the the MobileConfig to ReactFeatureFlags.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D27978112
fbshipit-source-id: 47e1e74398c62755bb0fdc6b54b7fd3aa47eb877
Summary:
## Motivation
With the bridge, every call into JS flushes the queue of NativeModule calls. Fabric bypasses this mechanism, because it uses a RuntimeExecutor that schedules work directly on the JavaScript thread. This diff makes Fabric's RuntimeExecutor also flush the queue of NativeModule calls.
This likely won't fix anything in Fabric, because we don't execute any async NativeModule calls on Fb4a. However, this is necessary for the drainMicrotask work we're doing in D27729702 (https://github.com/facebook/react-native/commit/73108477589a18cecb303ef556fa3da02f8ca1b8), because (1) we need to drain the Hermes microtask queue on every call from C++ -> JavaScript (2) we drain the microtask queue [inside JSIExecutor::flush()](https://github.com/facebook/react-native/blob/de477a0df6da770e579892d4875a8995c430ebdf/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp#L427,L445).
Changelog: [Android][Fixed] - Flush JSIExecutor in Fabric's RuntimeExecutor
Reviewed By: JoshuaGross
Differential Revision: D27975839
fbshipit-source-id: 27f031fb36593253da116a033e30998475eb1473
Summary:
By the time that we call [ReactContext.assertOnNativeModulesQueueThread()](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java?commit=747a25280435d276fb975ccfe36fd5e60254c4e4&lines=355%2C359), ReactContext.mNativeModulesMessageQueueThread must be non-null. This implies that two things must have happened:
1. We initialized the ReactContext
2. After initialization, ReactContext.mNativeModulesMessageQueueThread must be non-null.
According to T85807990, ReactContext.mNativeModulesMessageQueueThread is null. Since ReactContext doesn't ever write to ReactContext.mNativeModulesMessageQueueThread aside from during initialization, it must mean that we either didn't initialize properly, or we initialized, but set the NativeModules thread to null. This diff throws IllegalStateExceptions inside ReactContext initialization, which should help narrow down the crash in T85807990.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D27729355
fbshipit-source-id: e39030b7db8862ae76fb644efaafb382a79b8ad0
Summary:
Previously I renamed hasActiveCatalystInstance() API in D27335055 (https://github.com/facebook/react-native/commit/dfa8eb0558338f18ea01f294a64d355f6deeff06), however this API is still used in an OSS class.
In this diff hasActiveCatalystInstance() is added back and marked as Deprecated to avoid breakages and leave the migration choice to OSS users.
Changelog:
[Android][Changed] Mark hasActiveCatalystInstance() as Deprecated
Reviewed By: yungsters
Differential Revision: D27538449
fbshipit-source-id: 30f2f890580ad7f8b41908e18013234b5bac72e9
Summary:
Sometimes ```hasActiveCatalystInstance()``` is used to check if it's safe to access the CatalystInstance, which will still crash in Venice.
Previously we mitigate this by changing ```reactContext.hasActiveCatalystInstance()``` to ```reactContext.hasActiveCatalystInstance() || reactContext.isBridgeless()```.
To solve this for all and good the plan is:
1, Rename ```hasActiveCatalystInstance()``` to ```hasActiveReactInstance()``` so it won't sounds like CatalystInstance-only.
2, Implement hasActiveReactInstance() for Venice. D27343867
3, Remove previous mitigation. D27343952
This diff is the first step, by xbgs there are **58** non-generated callsites of ```hasActiveCatalystInstance()``` in code base which are all renamed in this diff.
Changelog:
[Android][Changed] - Rename "hasActiveCatalystInstance" to "hasActiveReactInstance"
Reviewed By: mdvacca
Differential Revision: D27335055
fbshipit-source-id: 5b8ff5e09b79a492e910bb8f197e70fa1360bcef
Summary:
Media picking wasn't working for Venice because we didn't implement onActivityResult in BridgelessReactFragment so the listener in FBProfileGemstoneReactModule didn't called.
Changelog:
[Android][Changed] - Added Nullable annotation
Reviewed By: mdvacca
Differential Revision: D27045861
fbshipit-source-id: 0ab2961ef0570d92259856b4132507ebb264eb9d
Summary:
## Rationale
The CatalystInstance is going away after we delete the bridge. So, we should migrate away from onCatalystInstanceDestroy() to something else: invalidate().
## Changes
- Introduce the NativeModule.invalidate() cleanup hook.
- Both the NativeModule and TurboModule infra now call this invalidate() method, **as opposed to** onCatalystInstanceDestroy(), to perform NativeModule cleanup.
- **Is this safe?** All our NativeModules extend BaseJavaModule. BaseJavaModule.invalidate() delegates to NativeModule.onCatalystInstanceDestroy(), so NativeModules that implement onCatalystInstanceDestroy(), but not invalidate(), still have their onCatalystInstanceDestroy() method called.
Changelog: [Android][Deprecated] - Deprecate NativeModule.onCatalystInstanceDestroy() for NativeModule.invalidate()
Reviewed By: JoshuaGross
Differential Revision: D26871001
fbshipit-source-id: e3bdfa0cf653ecbfe42791631bc6229af62f4817
Summary:
Simplify addLifecycleEventListener for the flaky test because we just want to test that listener is working.
Changelog:
[Android][Changed] - Add a spare implementation of addLifecycleEventListener for test purpose.
Reviewed By: PeteTheHeat
Differential Revision: D26749256
fbshipit-source-id: 5af216e6bfa37a15eb189aa24a3df35a7a7112de
Summary:
The purpose of this change is to make TTRC markers work similarly for bridge loading and bridgeless loading so we could compare performance between them.
There are mainly four cases involved:
```REACT_BRIDGE_LOADING_START,
REACT_BRIDGE_LOADING_END,
REACT_BRIDGELESS_LOADING_START,
REACT_BRIDGELESS_LOADING_END
```
First 2 are for beginning/ending of bridge loading which includes creating fragment, loading JS bundle, running JS bundle and creating react instance. Last 2 are for similar purpose with bridgeless.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D26514499
fbshipit-source-id: 65d6f3cc7de9e07a7a3a802dd77138e74c23aa5b
Summary:
This diff cleans listeners on the destruction of the ReactContext.
changelog: [inernal] internal
Reviewed By: JoshuaGross
Differential Revision: D26259929
fbshipit-source-id: 1843cabdac2fa3e67dcc890afd923b82472d8f66
Summary:
This diff clears the internal maps of NativeModuleRegistry during turn down of the bridge.
This is necessary for a proper cleanup of these modules.
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D26239303
fbshipit-source-id: 6e98e5db60a4f54d02e99b03339b03c17ecc183d
