Summary: ComponentScript uses Dimensions, but doesn't support native modules, so we need to keep the `nativeExtensions` stuff that was dropped in D16525189.
Reviewed By: PeteTheHeat
Differential Revision: D16611233
fbshipit-source-id: c0add40529743e02ab7943814dc9f2188e8e0633
Summary:
In order to cleanup the callsites that are not using Animated's native driver, we are going to make useNativeDriver a required option so people have to think about whether they want the native driver or not.
I made this change by changing [Animated.js](https://fburl.com/ritcebri) to have this animation config type:
```
export type AnimationConfig = {
isInteraction?: boolean,
useNativeDriver: true,
onComplete?: ?EndCallback,
iterations?: number,
};
```
This causes Flow to error anywhere where useNativeDriver isn't set or where it is set to false.
I then used these Flow errors to codemod the callsites.
I got the location of the Flow errors by running:
```
flow status --strip-root --json --message-width=0 | jq '.errors | [.[].extra | .[].message | .[].loc | objects | {source: .source, start: .start, end: .end}]'
```
And then ran this codemod:
```
const json = JSON.parse('JSON RESULT FROM FLOW');
const fileLookup = new Map();
json.forEach(item => {
if (!fileLookup.has(item.source)) {
fileLookup.set(item.source, []);
}
fileLookup.get(item.source).push(item);
});
export default function transformer(file, api) {
const j = api.jscodeshift;
const filePath = file.path;
if (!fileLookup.has(filePath)) {
return;
}
const locationInfo = fileLookup.get(filePath);
return j(file.source)
.find(j.ObjectExpression)
.forEach(path => {
if (
path.node.properties.some(
property =>
property != null &&
property.key != null &&
property.key.name === 'useNativeDriver',
)
) {
return;
}
const hasErrorOnLine = locationInfo.some(
singleLocationInfo =>
singleLocationInfo.start.line === path.node.loc.start.line &&
Math.abs(
singleLocationInfo.start.column - path.node.loc.start.column,
) <= 2,
);
if (!hasErrorOnLine) {
return;
}
path.node.properties.push(
j.property(
'init',
j.identifier('useNativeDriver'),
j.booleanLiteral(false),
),
);
})
.toSource();
}
export const parser = 'flow';
```
```
yarn jscodeshift --parser=flow --transform addUseNativeDriver.js RKJSModules react-native-github
```
Followed up with
```
hg status -n --change . | xargs js1 prettier
```
Reviewed By: mdvacca
Differential Revision: D16611291
fbshipit-source-id: 1157587416ec7603d1a59e1fad6a821f1f57b952
Summary:
## The Problem
1. `CatalystInstanceImpl` indirectly holds on to the `jsi::Runtime`. When you destroy `CatalystInstanceImpl`, you destroy the `jsi::Runtime`. As a part of reloading React Native, we destroy and re-create `CatalystInstanceImpl`, which destroys and re-creates the `jsi::Runtime`.
2. When JS passes in a callback to a TurboModule method, we take that callback (a `jsi::Function`) and wrap it in a Java `Callback` (implemented by `JCxxCallbackImpl`). This Java `Callback`, when executed, schedules the `jsi::Function` to be invoked on a Java thread at a later point in time. **Note:** The Java NativeModule can hold on to the Java `Callback` (and, by transitivity, the `jsi::Function`) for potentially forever.
3. It is a requirement of `jsi::Runtime` that all objects associated with the Runtime (ex: `jsi::Function`) must be destroyed before the Runtime itself is destroyed. See: https://fburl.com/m3mqk6wt
### jsi.h
```
/// .................................................... In addition, to
/// make shutdown safe, destruction of objects associated with the Runtime
/// must be destroyed before the Runtime is destroyed, or from the
/// destructor of a managed HostObject or HostFunction. Informally, this
/// means that the main source of unsafe behavior is to hold a jsi object
/// in a non-Runtime-managed object, and not clean it up before the Runtime
/// is shut down. If your lifecycle is such that avoiding this is hard,
/// you will probably need to do use your own locks.
class Runtime {
public:
virtual ~Runtime();
```
Therefore, when you delete `CatalystInstanceImpl`, you could end up with a situation where the `jsi::Runtime` is destroyed before all `jsi::Function`s are destroyed. In dev, this leads the program to crash when you reload the app after having used a TurboModule method that uses callbacks.
## The Solution
If the only reference to a `HostObject` or a `HostFunction` is in the JS Heap, then the `HostObject` and `HostFunction` destructors can destroy JSI objects. The TurboModule cache is the only thing, aside from the JS Heap, that holds a reference to all C++ TurboModules. But that cache (and the entire native side of `TurboModuleManager`) is destroyed when we call `mHybridData.resetNative()` in `TurboModuleManager.onCatalystInstanceDestroy()` in D16552730. (I verified this by commenting out `mHybridData.resetNative()` and placing a breakpoint in the destructor of `JavaTurboModule`). So, when we're cleaning up `TurboModuleManager`, the only reference to a Java TurboModule is the JS Heap. Therefore, it's safe and correct for us to destroy all `jsi::Function`s created by the Java TurboModule in `~JavaTurboModule`. So, in this diff, I keep a set of all `CallbackWrappers`, and explicitly call `destroy()` on them in the `JavaTurboModule` destructor. Note that since `~JavaTurboModule` accesses `callbackWrappers_`, it must be executed on the JS Thread, since `createJavaCallbackFromJSIFunction` also accesses `callbackWrappers_` on the JS Thread.
For additional safety, I also eagerly destroyed the `jsi::Function` after it's been invoked once. I'm not yet sure if we only want JS callbacks to only ever be invoked once. So, I've created a Task to document this work: T48128233.
Reviewed By: mhorowitz
Differential Revision: D16589168
fbshipit-source-id: a1c0786999c22bef55d416beb0fc40261447a807
Summary: When you create a TurboModule from the JS side, we instantiate its Java class and simply make this `javaobject` a `jni::global_ref` in C++. But the reason why we need to make this a global ref is because `JavaTurboModule` needs it to be a global reference for method calls. Making this a `jni::global_ref` from the perspective to TurboModuleManager doesn't really make any sense. So, this diff refactors that bit of code.
Reviewed By: mdvacca
Differential Revision: D16555673
fbshipit-source-id: 2778fc5a372c41847e8296c2e22bb9a8826fcc52
Summary:
## Description
When we reload the app, `ReactApplicationContext.destroy()` is called. This method calls `CatalystInstanceImpl.destroy()`, which (on the NativeModules thread) calls `NativeModuleRegistry.notifyJSInstanceDestroy()`, which loops over all its `ModuleHolder`s, and calls `ModuleHolder.destroy()`, each of which call their NativeModule's `onCatalystInstanceDestroy()` method.
TurboModuleManager is a `JSIModule`, so it also has its `onCatalystInstanceDestroy()` method called when the `ReactApplicationContext` is destroyed. But `TurboModuleManager.onCatalystInstanceDestroy()` doesn't do anything. Instead, at the very least, it should call `onCatalystInstanceDestroy()` of all NativeModules.
Reviewed By: mdvacca
Differential Revision: D16552730
fbshipit-source-id: 04459185262e92d69570facb368578a848cc5fdb
Summary: On iOS, calling the `__turboModuleProxy` function with the same name returns the same instance of the TurboModule. Adding this behaviour to Andorid as well.
Reviewed By: mdvacca
Differential Revision: D16553363
fbshipit-source-id: c95e150d6967604a808cfb49877b7a633e33d729
Summary:
Previously codegenNativeCommands was just a hint to the babel transform. This meant that in order to use the codegen'd JS command functions it required having the babel transform turned on.
We aren't ready to turn the transform on for open source so we are adding runtime behavior to the function that will run when it isn't replaced with the transform.
Reviewed By: rickhanlonii
Differential Revision: D16574781
fbshipit-source-id: 583e8857f69ae1695445ee887432d15248dd35a9
Summary:
Original commit changeset: 34a8f8395ca7
The problem with the original commit was the usage of optional chaining. This diff removes the usage of optional chaining with good old fashioned null checks.
Reviewed By: rickhanlonii
Differential Revision: D16593623
fbshipit-source-id: d24cc40c85de9a2e712e5de19e9deb196003ccf2
Summary: This fixes the lint that was broken by D16543439
Reviewed By: fkgozali
Differential Revision: D16607092
fbshipit-source-id: 70c3989d25f446ffe0877e58253acef5ada8e010
Summary:
This diff contains bunch of minor and straightforward fixes which need to be shipped together. They are all related to integrating objCpp codegen and compiling examples.
#Facebook
I explain in comments my thoughts
Reviewed By: RSNara
Differential Revision: D16520560
fbshipit-source-id: 15392017a92f5a7ec5da71b552ec6c6904625a86
Summary:
View Pooling is not currently being used in Fabric Android, this diff removes all the extra abstractions that are being used becuase of the unused ViewPooling.
We might add this in the future when we re-implement view correctly.
Reviewed By: JoshuaGross
Differential Revision: D16543439
fbshipit-source-id: f41b6e02fddc36c7ef7a1052399d2e6b2041fcfb
Summary:
Duplicate category method implementations cause undefined behavior in Objective-C; one is chosen essentially at random. The linker also issues a noisy warning about this (which is how I noticed this case).
It didn't matter in this particular case since both implementations do the same thing, but we should clean this up so people don't get desensitized to these linker warnings. There is no need to have two implementations.
Reviewed By: fkgozali
Differential Revision: D16587219
fbshipit-source-id: 56dc3493735443c476484092f4a7eacfcddee8cb
Summary:
Added an array to maintain the counts of each of the reason of measure callbacks
and this is now added as qpl metadata in Layout Calculation qpl event
Reviewed By: davidaurelio
Differential Revision: D16516379
fbshipit-source-id: 201c5d2463f0a921841a0bbfec8f4d5e007000c8
Summary:
We had flex as a reason for both layout and measure. Now creating separating reason flexLayout and flexMeasure in this diff.
Also changed ordering of items in Enum to group layout and measure reasons
Reviewed By: davidaurelio
Differential Revision: D16562350
fbshipit-source-id: 75501f9d4dde0974009193b3991a8acc97b02ad0
Summary: In Dev Settings, we used to have an `Start Sampling Profiler on init` option, which was defunct. This diff re-enables that option. We can now start the Sampling Profiler on app start
Reviewed By: yinghuitan
Differential Revision: D7022382
fbshipit-source-id: 1db85d8a324e401c71187ba5871a91abcc18acf9
Summary: `parseErrorStack` (which uses the `stacktrace-parser` package) can return null file names, line numbers and column numbers. This diff updates the associated types and adds explicit null checks in some call sites.
Reviewed By: rickhanlonii
Differential Revision: D16542176
fbshipit-source-id: b72c73c05b95df0bbcb5b5baa7bc2d42cff1e074
Summary:
For other platforms such as React VR it doesn't make sense to use `IntentAndroid` native module and it should use `LinkingManager` instead.
The code used to be:
```
const LinkingManager =
Platform.OS === 'android'
? NativeModules.IntentAndroid
: NativeModules.LinkingManager;
```
This diff changes the behaviour back to what it used to be.
Reviewed By: cpojer
Differential Revision: D16561073
fbshipit-source-id: 544551f8ff1affca5a71835133e8a9e7abc75e1a
Summary: While adding support for this to React VR I noticed that `viewConfig.Manager` was `undefined` which meant that the view config never get assigned to the `viewManagerConfigs` object and caused errors later on. This change makes it so that even if `viewConfig.Manager` is not set the viewConfig still gets added to the `viewManagerConfigs` object.
Reviewed By: rickhanlonii
Differential Revision: D16560992
fbshipit-source-id: 626dc133602b142caff60f41d043d02968e6ccfc
Summary:
# Context
In https://github.com/facebook/react/pull/16141 we imported `ReactFiberErrorDialog` unchanged from React. That implementation was not idempotent: if passed the same error instance multiple times, it would amend its `message` property every time, eventually leading to bloat and low-signal logs.
The message bloat problem is most evident when rendering multiple `lazy()` components that expose the same Error reference to React (e.g. due to some cache that vends the same rejected Promise multiple times).
More broadly, there's a need for structured, machine-readable logging to replace stringly-typed interfaces in both the production and development use cases.
# This diff
* We leave the user-supplied `message` field intact and instead do all the formatting inside `ExceptionsManager`. To avoid needless complexity, this **doesn't** always have the exact same output as the old code (but it does come close). See tests for the specifics.
* The only mutation we do on React-captured error instances is setting the `componentStack` expando property. This replaces any previously-captured component stack rather than adding to it, and so doesn't create bloat.
* We also report the exception fields `componentStack`, unformatted `message` (as `originalMessage`) and `name` directly to `NativeExceptionsManager` for future use.
Reviewed By: cpojer
Differential Revision: D16331228
fbshipit-source-id: 7b0539c2c83c7dd4e56db8508afcf367931ac71d
Summary: This test didn't get updated to run over all the fixtures like the others
Reviewed By: JoshuaGross
Differential Revision: D16501494
fbshipit-source-id: 4a68d60819701a9de7f1da22a66ccf807cc4490f
Summary:
This diff changes the name of the DispatchCommand methods in the UIManagerDelegate and Scheduler classes.
The purpose of this change is to use a consistent naming with the rest of the methods od these classes.
We might re-name these interfaces later, but for now we want to keep a consistent naming. For more details see discussion in D16543437
Reviewed By: JoshuaGross
Differential Revision: D16575403
fbshipit-source-id: 8335be8dc3367372a8ef2d7e8ed78665f4c02699
Summary:
`createReactContextInBackground` must be run on the UI Thread, so we update the DI annotation from `DefaultIdleExecutor` to `ForUiThread` so that happens.
Unclear why Infer didn't flag this. Asked [here](https://fb.workplace.com/groups/572076376174315/permalink/2661423100572955/).
Also adds an additional thread assert before setting `mHasStartedCreatingInitialContext = true` so that if a similar bug happens in the future, RN won't be completely hosed.
Reviewed By: mdvacca
Differential Revision: D16574901
fbshipit-source-id: 02ba63979904e9df9ef6d782aa7379cb44702508
Summary:
We want to enable codegenNativeCommands to have a runtime fallback that will work if the babel transform is not enabled. For example, in open source until we turn it on everywhere. By listing the supported commands, we can create the necessary functions at runtime to support what we need.
A follow up diff will add that runtime behavior to codegenNativeCommands.
Reviewed By: JoshuaGross
Differential Revision: D16573450
fbshipit-source-id: 189754a567a3a5ccd34629a8dfedf808e6824e82
Summary: This name was incorrect and didn't exist
Reviewed By: JoshuaGross
Differential Revision: D16528973
fbshipit-source-id: 92b874a2b023ff89ddc4312477ab8c91c0ef2cf6
Summary: Had to split it out if Jest because of tomocchino's patents. I hate it. Anyway, I brought it back into RN because I can't live without it.
Reviewed By: rubennorte
Differential Revision: D16560078
fbshipit-source-id: c394e248dcd048866a31a7b08b233d8966af9ee3
Summary:
The activityIndicatorViewStyle property overrides the previous set color
if it's changed. Depending on the property set order you may end in a state
that the color property will never be respected since it first sets
the color and then the activityIndicatorViewStyle property (which overrides
the color property). In order to prevent this problem
before setting the new activityIndicatorViewStyle save the old
color and override it after activityIndicatorViewStyle is set. Thus
always respecting the user's color.
## Changelog
[iOS] [Fixed] - Do not override ActivityIndicator color when setting its size
Pull Request resolved: https://github.com/facebook/react-native/pull/25849
Test Plan:
Using the code below on iOS notice that the last ActivityIndicator will always have its color set to white while te testID is provided
### Without the patch
Notice the white -> blue transition when disabling the testID
### With the patch
Color remains unchanged
```javascript
import React from "react";
import { View, StyleSheet, ActivityIndicator, Button } from "react-native";
const App = () => {
const [enableTestID, onSetEnableTestID] = React.useState(true);
const onPress = React.useCallback(() => {
onSetEnableTestID(!enableTestID);
}, [enableTestID]);
return (
<View style={styles.container}>
<ActivityIndicator size="large" color="red" />
<ActivityIndicator size="small" color="red" />
<ActivityIndicator size="small" />
<ActivityIndicator color="green" />
<ActivityIndicator
key={enableTestID.toString()}
size="large"
color="blue"
testID={enableTestID ? 'please work' : undefined}
/>
<Button
title={enableTestID ? 'Disable testID' : 'enable testID'}
onPress={onPress}
/>
</View>
);
};
export default App;
const styles = StyleSheet.create({
container: {
flex: 1,
alignItems: "center",
justifyContent: "center",
backgroundColor: "black"
},
});
```
Closes https://github.com/facebook/react-native/issues/25319
Reviewed By: cpojer
Differential Revision: D16559929
Pulled By: sammy-SC
fbshipit-source-id: ac6fd572b9f91ee5a2cbe46f8c46c1f46a1ba8b3
Summary: For Litho interop and to resolve T47926405, always pass props and state to Create mount item so that any ViewManager can create view instances with knowledge of initial props and state.
Reviewed By: mdvacca
Differential Revision: D16554082
fbshipit-source-id: 3b19a43347b0fa201a054eec60e82fb77cad3625
Summary: Add a test to ensure that the generated code is syntactically valid and compiles.
Reviewed By: JoshuaGross, osdnk
Differential Revision: D16518542
fbshipit-source-id: d4fadaeb29194ca38c8a99874ab304a464632894
Summary: Support command arguments that are strings
Reviewed By: JoshuaGross
Differential Revision: D16509728
fbshipit-source-id: 003aba66231d204071d043c01cb0781150d0edb9
Summary: I want a fixture with all the types, so refactoring this so future diffs in the stack are cleaner
Reviewed By: JoshuaGross
Differential Revision: D16509803
fbshipit-source-id: 1f4873701a8ff842f50976377003e1abff187278
Summary:
These functions will be called by components like this:
```
- (void)handleCommand:(NSString const *)commandName args:(NSArray const *)args
{
ViewNativeComponentHandleCommand(self, commandName, args);
}
```
Codegen currently supports commands with ints and bools as arguments. Will add more types in follow up diffs.
Reviewed By: JoshuaGross
Differential Revision: D16509123
fbshipit-source-id: c3071ce3b5da215bb8747216e57026a69a89eff0
Summary: We will be generating protocols for every component even if there are no commands. This is for consistency and our ability to add to them later without changing every native component that doesn't currently have a command. JoshuaGross and I figure this is okay as it appears that empty protocols are very cheap on app size
Reviewed By: zackargyle, JoshuaGross
Differential Revision: D16503773
fbshipit-source-id: 11b78fcd33b68926def909d3ce42f58b9bbee96a
Summary:
# Disclaimer:
I might be missing something as the solution I implemented here seems like something that was considered by original author. If this solution isn't good, I have a plan B.
# Problem:
`onDismiss` prop isn't being called once the modal is dismissed, this diff fixes it.
Also I've noticed that `onDismiss` is meant to only work on iOS, why is that? By landing this diff, it'll be called on Android as well so we need to change the docs (https://facebook.github.io/react-native/docs/modal.html#ondismiss).
## Video that shows the problem
Following code is in playground.js P70222409 which just increments number everytime onDismiss is called
{F166303269}
Reviewed By: shergin
Differential Revision: D16109536
fbshipit-source-id: 3fba56f5671912387b217f03b613dffd89614c9d
Summary:
Automatically provides and subscribes to dimension updates - super easy usage:
```
function MyComponent(props: Props) {
const {width, height, scale, fontScale} = useWindowDimensions();
return <Text ...
};
```
Only window for now - it's what people want 99% of the time, so we'll just shovel out a pit of success for them...
There are still cases where `Dimensions` is needed outside of React component render functions, like in GraphQL variables, so we need to keep the existing module.
Reviewed By: zackargyle
Differential Revision: D16525189
fbshipit-source-id: 0a049fb3be8d92888a8a69e3898d337b93422a09
Summary: I think it's possible that there's a race condition between creating the scheduler and setting the delegate leading to bugs like T47272192.
Reviewed By: mdvacca
Differential Revision: D16537737
fbshipit-source-id: 9c579537658be5a9aeed37c0e4935c997cabb6aa
Summary:
This fixes a bug where a ref to a class would get nulled after the class is edited. Now it's appopriately updated.
This is technically a partial sync on top of my last cherry-picked one. It only picks up this commit: https://github.com/facebook/react/commit/9914a19190296ca7e0fd65f7c4f6fe5cc42e29a4. The changes are DEV-only and only affect Fast Refresh.
Reviewed By: motiz88
Differential Revision: D16543751
fbshipit-source-id: c1fc393e78d0e13070721037d16734c9ece38bc9
Summary:
# Problem:
I enabled Fabric for "Profile About", when I navigate to "Marketplace Home" from "Profile About", I get following warning "<Component> is not Fabric compatible yet". Even though the component has been migrated to Fabric and is getting registered. The component works if I navigate straight to "Marketplace Home".
{F172219008}
# How it was solved:
It turns out, there was a missing call that added newly created registry to `componentDescriptorRegistries_`.
Reviewed By: shergin
Differential Revision: D16542109
fbshipit-source-id: 22a315333cfff0895ce77e8439816f4e7bf34d35
Summary:
Changelog:
[General] [Changed] - Include only Java classes in zip archive of generated files for JS codegen
This diff adds one more Buck rule to copy only Java files to a temporary dir, so that the zip_rule will only package the Java classes and skip C++ files. It makes the generated code more organized and decreases the time to create the ZIP-archive.
Reviewed By: rickhanlonii
Differential Revision: D16540781
fbshipit-source-id: cbc99b5fe28b6af5375a88652cefb30e672ed527
Summary:
It appears that Electron (or the version of Chromium it uses) has a bug that causes a `webview` process to crash if `URL.createObjectURL` is used.
Before releasing `react-devtools-core` 3.5.0, we updated Webpack and the loaders we used. Apparently the version of `style-loader` we now use makes use of the `URL.createObjectURL` API for CSS source maps. This triggers the `webview` crash I mentioned above.
The fix for this is to disable CSS source maps, in which case the loader just uses a `<style>` tag. This diff updates Nuclide to pull in this fixed version.
Reviewed By: bestander
Differential Revision: D16518772
fbshipit-source-id: a779b7d310f869793fa05988d138ce6a46840d8c
Summary: In this this diff I follow with codegening constants structs.
Reviewed By: RSNara
Differential Revision: D16496128
fbshipit-source-id: e4140d97b378985502911b8dcd1723f153dabf00
Summary:
I am sending an asynchronous function as callback to the `.start` method of `Animation.parallel([...]).start(callback)`. Flow does not like this, as the `EndCallback` type is saying that these callbacks must return `void`. Since my callback returns `Promise<void>` this results in an error.
Does it really matter what the callback returns?
## Changelog
[General] [Changed] - Make Animation EndCallback type allow any return value
Pull Request resolved: https://github.com/facebook/react-native/pull/25793
Test Plan: I have run `yarn flow`, which reported no errors.
Reviewed By: cpojer
Differential Revision: D16515465
Pulled By: osdnk
fbshipit-source-id: 420d29d262b65471e6e1ad4b5a126bf728336260
