Summary:
This diff refactors the theme management for text input in order to avoid extra state updates.
changelog:[Internal]
Reviewed By: JoshuaGross
Differential Revision: D22149754
fbshipit-source-id: 8a6dbe63c8d532986dbf785c7b16323e0a980669
Summary:
Index adjustment is complicated since we defer REMOVE operations. This diff solves some remaining (hopefully final!) bugs found when queueing several conflicting LayoutAnimations repeatedly (this is sort of an "extreme" case since we're continuously re-queueing additional LayoutAnimations instead of allowing any to complete, while reordering and deleting, so it exercises the most extreme LayoutAnimations use-cases).
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22150085
fbshipit-source-id: db1381bef3ba8bb5ec82dab791f69162d46d23f0
Summary:
This will prevent views from becoming out-of-order as view removals are delayed and there are inserts at the same level.
There is at least one additional issue that crops up if many animations are queued up at the same time, that will be resolved separately.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D22130185
fbshipit-source-id: d8041c6afdcd729939dd392c9e2c23fe8ec1b706
Summary:
Changelog: [Internal]
Order of events dispatched to JS matter. If we remove duplicate events, we shouldn't change order of types.
So If we have order of events
A -> B -> C
and inserting a new event A for the same target, we shouldn't delete the first A, as there might be logic that depends on event B being preceded by A.
What we can do though, is delete previous event of the same type and target in queue if we would be adding subsequent event.
So when we have events
A -> B -> C
and we are about to add event C to the queue which goes to the same target, we can delete the first C before inserting the new one.
Reviewed By: shergin
Differential Revision: D22135931
fbshipit-source-id: 1dadd1688516330be07c4251f446f77ca08eaa87
Summary:
When asking for the data of a JSBigFileString, check that the instance satisfies some basic invariants.
This is meant to catch any corruption issues as early as possible.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22125436
fbshipit-source-id: e0a84752c86151d56b7e7cbed3b95650d8ba1f75
Summary:
Add tooltip to the list of components supported with the legacy view manager, so that it will work in Fabric.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D21917137
fbshipit-source-id: 1ab6c7cdd0a10a496531d2ea8fbcae7e427ec647
Summary:
Changelog: [Internal]
Add support for dynamic font size.
New class `ThreadStorage` is introduced, which is used to pass LayoutContext to `YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector`.
## Shortcoming
This implementation doesn't cause re-render, if user changes font size and comes to the app without restarting it, it will show old font size. I believe this is fine for now as most people set their font size before they use the app and keep the same setting for a long time.
Reviewed By: shergin
Differential Revision: D22043728
fbshipit-source-id: 7453d165c280a2f4bcb73f4ee6daf9e64b637ded
Summary:
Changelog: [internal]
Rename `LayoutableShadowNode::measure` to `LayoutableShadowNode::measureContent` and add LayoutContext as parameter.
Pass `LayoutConstraints` by reference rather than value.
Reviewed By: shergin
Differential Revision: D22043727
fbshipit-source-id: b668240c45b658db5b114630b73d7407d35482aa
Summary:
Changelog: [Internal]
A long time ago we experimented with JSC bytecode. We are not experimenting with JSC bytecode any more. This code can be removed.
Reviewed By: mhorowitz
Differential Revision: D22017374
fbshipit-source-id: 6fe3fb7ad7966f92a5cd103605ac5c0bd1f17a8e
Summary:
Fix crashes in LayoutAnimations:
1) It is valid to omit create, update, delete configs
2) When extracting SRT from a matrix, ignoring skew properties
3) Provide valid telemetry from LayoutAnimations transactions
Unrelated to crashes: to help debugging and until onSuccess/onError callbacks are working, log any configuration parsing errors.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22050736
fbshipit-source-id: e59418ecad0f9bfd20a2b4976557e39020c2d101
Summary:
This diff fixes a race condition that caused a flicker during initial rendering of TextInput in Fabric
The root cause is that the TextInput's State is sometimes initialized with no information from the Theme, this causes text input to be rendered with 0 padding. Later ReactTextInput manager updates TextInput state with theme data and the TextInput is re-rendered with the correct padding information.
changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D22034849
fbshipit-source-id: a2fa91f34a8340878f2ec8d231ef6c86cee08f05
Summary:
This pull request updates the Podspecs to reflect recent changes to Fabric, so they build with Fabric enabled again.
In `React-Fabric.podspec`, subspecs for the views `legacyviewmanagerinterop`, `safeareaview`, `textinput`, and `unimplementedview` are added. A minor tweak to the subspec for `rncore` was added blacklisting two codegen-generated files that have issues building, but are not actually needed yet and are empty.
In `React-graphics.podspec`, the header paths for Boost were added.
This is partially a retread of https://github.com/facebook/react-native/issues/26805, which had to be backed out due to issues with the changes to the Yoga podspec.
## Changelog
[Internal] [Fixed] - Fixed building Fabric with the Cocoapods
Pull Request resolved: https://github.com/facebook/react-native/pull/28584
Test Plan: RNTester can now compile.
Reviewed By: shergin
Differential Revision: D20966065
Pulled By: hramos
fbshipit-source-id: 73931be63b72fcb52705643f81c0f787e571eb76
Summary:
Changelog: [Internal]
Round calculated text size to closest larger pixel size.
In Paper we do this as well in https://fburl.com/diffusion/w2pj6ea0
Why do we need this?
For example, we calculate height of the text to be 16.707235, yoga takes this value and assigns text to have height 17 anyway.
I assume Yoga uses this extra 0.3 points of height to move other things around a little bit because Yoga doesn't deal with exact values.
Reviewed By: shergin
Differential Revision: D21999032
fbshipit-source-id: 73923dc3e27fa110adb3f76cfa635e0bfdc672d4
Summary:
There is no good reason why they are `const` and `private`, in the future diffs we will need to mutate them.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21992197
fbshipit-source-id: f8aee8db9ea1ff8282b38fbe3a966911678ee2c4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/29090
Moving the logic for calling into JS to handle errors into ErrorUtils, where it can be reused outside of the bridge.
Reviewed By: RSNara
Differential Revision: D21939254
fbshipit-source-id: 0d8f3bd2503720be7619ed8dc8b2389f544049f3
Summary:
While I was investigated the previous issue (T59430348) I found this place where we manipulate managed pointers manually. So, to make it error-prone this diff changes it to use `wrapManagedObject`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D21943010
fbshipit-source-id: ecaeffb419ae8d4880187027ca7ec9563e0dfd46
Summary:
ManagedObjectWrapper (`wrapManagedObject`) is used to pass pointers managed by Objective-C runtime object thought C++ parts of the framework. The wrapper consists of a shared pointer to an object with a custom deleter that calls `CFRelease`.
Apparently, there is a caveat here: shared ptr implementation always calls a deleter even if the object points to `nullptr`. It's usually fine because in C++ calling `delete` on a nullptr is a no-op, not an error. But it's an error from the `CFRelease` perspective, which checks the pointer and crashes if it's nullptr.
More info:
https://stackoverflow.com/questions/1135214/why-would-cfreleasenull-crashhttps://stackoverflow.com/questions/50201967/why-unique-ptr-with-custom-deleter-wont-work-for-nullptr-while-shared-ptr-does
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21943011
fbshipit-source-id: 442ad5e274a146de112e6bd8f3c2d20f0225bf77
Summary:
Changelog: [Internal]
`CFRelease` crashes app if `cf` is `NULL`, `CGColorRelease` doesn't.
From:
https://developer.apple.com/documentation/coregraphics/1586340-cgcolorrelease?language=objc#
Even though this hasn't happened yet, we had a similar crash in D21943011. Also `CGColorCreate` returns nullable so it could happen.
In order to prevent this in the future, let's switch to `CGColorRelease` which doesn't crash if `cf` is `NULL` and also is semantically correct.
Reviewed By: shergin
Differential Revision: D21953404
fbshipit-source-id: 8b969ee345c2507fcba2442fa20e3fdaf2235d8c
Summary:
Changelog: [Internal]
View gets flattened even though it has `display: none` and therefore it and its children do not get hidden.
Reviewed By: shergin, mdvacca
Differential Revision: D21929033
fbshipit-source-id: 994a79fb64fbe66273a70218ebe8056d92cd3cd4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/29087
D21908523 added an implicit dependency on `jsi.h` to use functions like `asObject`, etc. For some reason this doesn't break the build with BUCK (??) but it does with cocoapods. Adding the dep to the cxxreact podspec and regenerating offline mirrors to unbreak CircleCI. Also adding the BUCK dep and include statement for good measure.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21924592
fbshipit-source-id: 295c0670c6499e1195ba3c3a3320c6aee13bc025
Summary:
Changelog: [Internal]
The crash is caused by dereferencing invalid pointer.
This can happen because `UIManager` can outlive `RCTScheduler` and `Scheduler`.
Here we make sure when `Scheduler` is being deconstructed, UIManager's pointer is invalidated.
I don't think this is ideal solution but it should fix the crash.
Ideally we want the owner of animation delegate to invalidate the pointer.
Reviewed By: JoshuaGross
Differential Revision: D21922910
fbshipit-source-id: b2a56c1104574cecebaffad1bcbcbff82c1fa0cf
Summary:
Call into our existing JS error handling logic from C++ using JSI in the RuntimeExecutor used by the bridge.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D21908523
fbshipit-source-id: ae41196443781b9f2673dcb7bbcb5b5aa8aa2528
Summary:
Changelog: [Internal]
# Problem
`MountingCoordinator` holds a pointer to instance of `MountingOverrideDelegate` which becomes invalid.
# Solution
Use `std::weak_ptr` instead of raw pointer so it is possible to tell whether the pointer is expired.
Reviewed By: JoshuaGross
Differential Revision: D21905351
fbshipit-source-id: c7bf9635742a6ec086a03ba83202e46e1f1f373f
Summary:
This diff adds support elevation prop in Fabric core, additionally it adds this prop as non collapsable on the view flattening algorithm
changelog: [Internal] internal change in fabric to support elevation prop
Reviewed By: JoshuaGross
Differential Revision: D21896465
fbshipit-source-id: e0854acc0b2ac30eaf3f82d615aab1cf378cc530
Summary:
JSError creation can lead to further errors. Make sure these cases
are handled and don't cause weird crashes or other issues.
This solution has a few parts:
* include a ScopedNativeDepthTracker in checkStatus
* If an exception object message or stack property is already a String, don't
call JS String ctor on it
* Verify that a jsi::Value is a String before calling getString on it.
* Add more tests for JSError construction
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D21851645
fbshipit-source-id: 2d10da0e741ad4ede93cd806320f68ad512e5138
Summary:
There are two ways to make a NativeModule method execute synchronously:
- Declare the NativeModule method to be synchronous (i.e: use `RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD`).
- Make the NativeModule synchronous (i.e: make its method queue `RCTJSThread`). This way, all its methods are synchronous.
`RCTNativeModule` executes all synchronous methods on the JS thread:
- Executing an async methods on a sync module: https://git.io/JfRPj
- Executing a sync method: https://git.io/JfRXe
However, in TurboModules we block the JS thread, execute the method on the NativeModule's method queue, and then unblock the JS thread. While this approach is thread-safe, and arguably the correct way to dispatch sync methods, it's also much slower than the alternative. Therefore, this diff migrates the legacy behaviour to the TurboModule system.
## Special Case: getConstants()
When an ObjC NativeModule requires main queue setup, and it exports constants, we execute its `constantsToExport` method on the main queue (see: [RCTModuleData gatherConstants](https://github.com/facebook/react-native/blob/c8d678abcf93fd3f6daf4bebfdf25937995c1fdf/React/Base/RCTModuleData.mm#L392-L402)). I replicated this behaviour with TurboModules.
Changelog:
[iOS][Fixed] - Execute ObjC TurboModule async method calls on JS thread for sync modules
Reviewed By: fkgozali
Differential Revision: D21602096
fbshipit-source-id: 42d07b7ad000abeac27091dc3ec440e3836d2eae
Summary:
If a NativeModule requires main queue setup, its `getConstants()` method must be executed on the main thead. The legacy NativeModule infra takes care of this for us. With TurboModules, however, all synchronous methods, including `getConstants()`, execute on the JS thread. Therefore, if a TurboModule requires main queue setup, and exports constants, we must execute its `getConstants()` method body on the main queue explicitly.
**Notes:**
- The changes in this diff should be a noop when TurboModules is off, because `RCTUnsafeExecuteOnMainQueueSync` synchronously execute its block on the current thread if the current thread is the main thread.
- If a NativeModule doens't have the `requiresMainQueueSetup` method, but has the `constantsToExport` method, both NativeModules and TurboModules assume that it requires main queue setup.
## Script
```
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 constantsToExport")
.split("\n")
.filter(Boolean);
const filesWithoutConstantsToExport = [];
const filesWithConstantsToExportButNotGetConstants = [];
const filesExplicitlyNotRequiringMainQueueSetup = [];
tmFiles
.filter((filename) => {
if (filename.includes("microsoft-fork-of-react-native")) {
return false;
}
return /\.mm?$/.test(filename);
})
.map(abspath)
.forEach((filename) => {
const code = readFile(filename);
const relFilename = relpath(filename);
if (!/constantsToExport\s*{/.test(code)) {
filesWithoutConstantsToExport.push(relFilename);
return;
}
if (!/getConstants\s*{/.test(code)) {
filesWithConstantsToExportButNotGetConstants.push(relFilename);
return;
}
if (/requiresMainQueueSetup\s*{/.test(code)) {
const requiresMainQueueSetupRegex = /requiresMainQueueSetup\s*{\s*return\s+(?<requiresMainQueueSetup>YES|NO)/;
const requiresMainQueueSetupRegexMatch = requiresMainQueueSetupRegex.exec(
code
);
if (!requiresMainQueueSetupRegexMatch) {
throw new Error(
"Detected requiresMainQueueSetup method in file " +
relFilename +
" but was unable to parse the method return value"
);
}
const {
requiresMainQueueSetup,
} = requiresMainQueueSetupRegexMatch.groups;
if (requiresMainQueueSetup == "NO") {
filesExplicitlyNotRequiringMainQueueSetup.push(relFilename);
return;
}
}
const getConstantsTypeRegex = () => /-\s*\((?<type>.*)\)getConstants\s*{/;
const getConstantsTypeRegexMatch = getConstantsTypeRegex().exec(code);
if (!getConstantsTypeRegexMatch) {
throw new Error(
`Failed to parse return type of getConstants method in file ${relFilename}`
);
}
const getConstantsType = getConstantsTypeRegexMatch.groups.type;
const getConstantsBody = code
.split(getConstantsTypeRegex())[2]
.split("\n}")[0];
const newGetConstantsBody = `
__block ${getConstantsType} constants;
RCTUnsafeExecuteOnMainQueueSync(^{${getConstantsBody
.replace(/\n/g, "\n ")
.replace(/_bridge/g, "self->_bridge")
.replace(/return /g, "constants = ")}
});
return constants;
`;
writeFile(
filename,
code
.replace(getConstantsBody, newGetConstantsBody)
.replace("#import", "#import <React/RCTUtils.h>\n#import")
);
});
console.log("Files without constantsToExport: ");
filesWithoutConstantsToExport.forEach((file) => console.log(file));
console.log();
console.log("Files with constantsToExport but no getConstants: ");
filesWithConstantsToExportButNotGetConstants.forEach((file) =>
console.log(file)
);
console.log();
console.log("Files with requiresMainQueueSetup = NO: ");
filesExplicitlyNotRequiringMainQueueSetup.forEach((file) =>
console.log(file)
);
}
if (!module.parent) {
main();
}
```
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D21797048
fbshipit-source-id: a822a858fecdbe976e6197f8339e509dc7cd917f
Summary:
At some early stages of Instance initialization, it does not have a `nativeToJsBridge_`. At the same time, `handleMemoryPressure` can be called at any point in time, so we should check the pointer for not being null before calling on it.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D21798522
fbshipit-source-id: 6384da88784cceb493cf9810408cbb47777d3f4b
Summary:
This diff extends the measurement of Text components in order to support empty strings.
This is required for parity with Paper.
I created a follow up task to analyze support of empty string as part of the Text infrastructure of Fabric in the future.
changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D21761171
fbshipit-source-id: d2aa074052b09732af5d35723f19014090fcabbf
Summary:
`getStackTracesTreeNodeForAlloc` only works for memory that was tracked with `newAlloc`.
For now, that is only memory that goes through GC APIs.
Various places were calling this function on native memory which was allocated
with `malloc`, not GC memory. This led to SIGSEGV on nullptr when trying to take heap profiles
of these objects.
`newAlloc` could, in theory, work with native memory as well. But building out support for that
is outside the scope of this fix.
Changelog: [Internal]
Reviewed By: jbower-fb
Differential Revision: D21667842
fbshipit-source-id: 8403a6668e5ec607972ce6819f78fedb89da3f37
Summary:
Index adjustment doesn't work if virtual views are inserted, because those don't actually result in the view hierarchy being mutated, as such.
Add an Android-specific check to solve the crash.
I don't love adding platform-specific checks here, so I'm considering wrapping this logic in a platform-specific class, so that logic lives outside of the core of LayoutAnimations and entirely in platform code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21727805
fbshipit-source-id: 5af2cf479beaa4d0e9d94ea16ac989c4268920f8
Summary:
property and type are optional params according to Flow, so we should treat as such.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21725746
fbshipit-source-id: 3c48a8cef8fa5911c195f582556de6dad871c4f1
Summary:
A key difference in WeakRefs with the Hades GC is that they are mutable,
in the sense that reading the value of a WeakRef while a GC is active might
clear the WeakRef.
For this reason, attempting to lock the WeakObject might mutate the backing
reference.
I preferred to communicate this change in behavior via the API rather than
`const_cast` it away inside the implementation.
Changelog: [Internal] Change jsi::WeakObject to be mutable in lockWeakObject
Reviewed By: mhorowitz
Differential Revision: D21485758
fbshipit-source-id: 3618928be8f8791aed56cb20673896ff5b786ded
Summary:
Simple diff to add ART components into Catalyst app Android
changelog: [Internal] Internal changes to add support of ART for Fabric
Reviewed By: JoshuaGross
Differential Revision: D21621479
fbshipit-source-id: d957c25f447d19d8f349c69aa20f5f19237d867a
Summary:
Here I'm implementing equality methods for ARTGroup, ARTShape and ARTText and I'm using these methods to update the state only when it is necessary.
This will improve perf in rendering of ART
changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D21695127
fbshipit-source-id: b438ddea4c34bd7a0bdf26a6aac4fd62a9f78b49
Summary:
Element, Shape, Group, Text are too generic, we are renaming these classes to ARTElement, ARTBGroup, ARTShape...
changelog: [Internal] internal changes to support ART in android
Reviewed By: JoshuaGross
Differential Revision: D21681878
fbshipit-source-id: f6b35443486a3db210f61dcaf91bd32df47fbc66
Summary:
This diff adds support for Text in ART Fabric Android
changelog: [Internal] internal changes to fully support ART in Fabric
Reviewed By: JoshuaGross
Differential Revision: D21681877
fbshipit-source-id: c92e642cff56b71f8ee8f4eb9af6eea6c490f6c7
Summary:
This diff implements the serialization of Text components to send data from C++ to java
changelog: [Internal] internal changes to support ART in fabric
Reviewed By: JoshuaGross
Differential Revision: D21681875
fbshipit-source-id: eba31f35c95e0a2d3226ec70421832719083d7fa
Samuel Susla
David Vacca
Joshua Gross
Daniel Andersson
Luna Wei
Kacie Bawiec
Christoph Nakazawa
empyrical
Valentin Shergin
Emily Janzer
Ryan Tremblay
Sidharth Guglani
Paige Sun
Marc Horowitz
Ramanpreet Nara
Riley Dulin
Christoph Purrer