Summary:
We recently updated React Native's docs site to have its own domain reactnative.dev and needed to update the URLs in the source code
CHANGELOG:
[INTERNAL]
Reviewed By: hramos
Differential Revision: D20072842
fbshipit-source-id: 1970d9214c872a6e7abf697d99f8f5360b3b308e
Summary:
Changelog: [internal]
Repro steps:
1. Go to fbsource/fbobjc.
2. Run following focus command P124860753.
3. Enable Address Sanitizer in Xcode.
3. Run the app from Xcode.
5. Make sure `react_fabric.enable_marketplace_home_ios` and `react_fabric.enabled_state_reconciliation_ios` Mobile Config flags are set to true.
6. Navigate to Marketplace Home and observe use of freed memory exception. You have to wait until second page of marketplace home feed is loaded.
{F228880980}
# Root cause
`ChangedShadowNodePairs` being a vector of pairs of references.
Those `shared_ptr` do not outlive the the run of `reconcileStateWithTree` (that's what I originally thought when I made them references).
This was introduced in D19814500.
Reviewed By: JoshuaGross
Differential Revision: D19977784
fbshipit-source-id: 8c4a381137baa08a6af9d3c1cb295fe6c2b2bf95
Summary:
Changelog: [Internal]
This allows to block queue until image is fetched.
In paper, the photo of dog will not appear unless the network request is "fast enough".
{F228670428}
Reviewed By: shergin
Differential Revision: D19907906
fbshipit-source-id: 98d68abf1255388f9b97fb9da367d8f583a4220d
Summary:
[A recent change to JSStringToSTLString](https://github.com/facebook/react-native/pull/26955) causes a crash when the function is invoked with invalid UTF-16 data. The old behaviour, restored here, was to truncate the string before the first invalid character.
Here's how [the original code](https://github.com/facebook/react-native/blob/aee88b6843cea63d6aa0b5879ad6ef9da4701846/ReactCommon/jsi/JSCRuntime.cpp#L287) handled this case:
```
std::string JSStringToSTLString(JSStringRef str) {
size_t maxBytes = JSStringGetMaximumUTF8CStringSize(str);
// ^ maxBytes >= 1 regardless of str's contents
std::vector<char> buffer(maxBytes);
// ^ vector is zero initialised
JSStringGetUTF8CString(str, buffer.data(), maxBytes);
// ^ writes '\0' at the first invalid character and returns early (see JSC source code)
return std::string(buffer.data());
// ^ copies the string up to the first '\0'
}
```
See the JSC implementations of [`JSStringGetUTF8CString`](https://opensource.apple.com/source/JavaScriptCore/JavaScriptCore-7600.8.7/API/JSStringRef.cpp.auto.html) and [`convertUTF16ToUTF8`](https://opensource.apple.com/source/WTF/WTF-7600.7.2/wtf/unicode/UTF8.cpp.auto.html).
Based on the fact that `JSStringGetUTF8CString` *always* null-terminates the buffer - even when it bails out of converting an invalid string - here we're able to both
1. keep the fast path (not zero-initialising, not scanning for the null terminator) for the common case when the data is valid and JSStringGetUTF8CString returns a nonzero length; and
2. return the truncated string when JSStringGetUTF8CString returns an error code of 0, by scanning for the null terminator.
Changelog: [General] [Fixed] - Fix crash when passing invalid UTF-16 data from JSC into native code
Differential Revision: D19902751
fbshipit-source-id: 06bace2719800e921ec115ad6a29251eafd473f6
Summary:
Having the overridden function that returns a different type is apparently not a good idea (and might cause bugs and unexpected behavior), so it was renamed. The function also got a new return type (`const &` instead of `std::shared_ptr`) for simplicity, better performance, and smaller code size.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19837694
fbshipit-source-id: b7a96424bd040409371724907b3fb3931cd8a2e8
Summary:
In all other areas of JSI, std::string is treated as potentially
containing UTF-8 bytes (instead of ASCII). This fixes the inconsistency.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D19871520
fbshipit-source-id: c703f07e10bedbf2518d0bec903f85f43bbcbdf5
Summary:
If we're using synth traces in production, we probably don't want to accumulate the trace in memory, and write it to a file in one big step when an error happens. Rather, we'd like to write the trace incrementally, to a file: file system space is less scarse than memory, and doing it incrementally means less to do in the error handler.
This diff is the first step towards writing the synth trace incrementally. The most difficult aspect of this is that in the existing code, on Android, the trace is written in the handler of an Intent. Java-level operations on the Intent yielded a temporary filename under the application (so if the application is uninstalled, these temporary files are deleted). We must do something similar, but choose the filename within native code, on construction of the SynthTrace object. We copy what profiling does for this: assume the tmp dir is /data/data/<app>, where <app> can be found from reading /proc/self/cmdLine.
The SynthTrace constructor now takes a new argument: a unique_pointer to a stream. If null, no trace file is written, and the SynthTrace just accumulates records in memory. (This functionality is used during trace replay.) . If non-null, the trace is written to that stream.
The "write{BridgeTraffic}Trace..." methods become "flushAndDisable{BridgeTrafficTrace}" methods.
This diff is the first step towards incremental traces: the trace is still written at the end, but we've arranged that the place to write it is available at construction of the SynthTrace object. Later diffs will move recording of static things (e.g., the RuntimeConfig) into the ctor, then actually make us write the trace to the file incrementally.
For ReactNative:
Changelog: [Internal]
Reviewed By: haozhun, dulinriley
Differential Revision: D19471297
fbshipit-source-id: c1de4d2d9f44a87c7ff6fea38a1ce67de593940c
Summary:
With new logic we save one attributed string copy operation.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19844371
fbshipit-source-id: 62f6b0f9c8514a1b55224ccaa52c8fa89c06c9e7
Summary:
Instead of returning a `bool` which gives no information about the cause of the error,
return `void` and throw when there's some error.
Another alternative is returning `std::error_code`, but that's less flexible than throwing, and
this API already supports throwing.
Changelog: [Internal]
Reviewed By: jbower-fb
Differential Revision: D19170033
fbshipit-source-id: 870cd996a1a53c94524455f31765c1da99f57a1d
Summary:
Changelog: [Internal]
There shouldn't be any logical changes to the code.
Reviewed By: JoshuaGross
Differential Revision: D19814500
fbshipit-source-id: 2be492eebb284c7e241662d9d78117badcb2eee4
Summary:
# Problem
We calculate `stateRevision_` in constructor from children, but children change after shadowNode is initialised and before it is sealed. So the `stateRevision_` we calculate in constructor can be incorrect.
# Solution
Recalculate `stateRevision_` whenever children change. This can happen in two methods `ShadowNode::replaceChild` and `ShadowNode::appendChild`. This diff implements this solution.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D19813840
fbshipit-source-id: 8fc3b6601e4618f4ee5b322eebc230e0bbb92e3a
Summary:
Before this change the Element<> API didn't allow to specify state objects and crashed during instantiation of non-null states.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19816358
fbshipit-source-id: 95ba5e03ea98c0260593462146e8523c95245e2b
Summary:
This pull request fixes a few `#include`s in the Fabric source:
* Changes `<Glog/logging.h>` to `<glog/logging.h>` in two files, which was an issue for case-sensitive file systems
* In `ParagraphProps.cpp`, changes the include of `attributedstring/primitives.h` from a relative style to a more absolute style.
## Changelog
[Internal] [Fixed] - Fabric: Fix various includes
Pull Request resolved: https://github.com/facebook/react-native/pull/27947
Test Plan: Fabric compiles perfectly, and the Fabric test suite passes.
Reviewed By: shergin
Differential Revision: D19788148
Pulled By: sammy-SC
fbshipit-source-id: 30a297e8aea50c7c7fd5f48745edf04811a6bdbd
Summary:
This pull request fixes the CXX stub for `TextLayoutManager`. The signature for `TextLayoutManager::measure` changed slightly, and this patch updates the stub to reflect that.
## Changelog
[Internal] [Fixed] - Fabric: Fix CXX Stub for TextLayoutManager
Pull Request resolved: https://github.com/facebook/react-native/pull/27948
Test Plan: Fabric compiles, and the Fabric test suite passes.
Reviewed By: shergin
Differential Revision: D19788207
Pulled By: sammy-SC
fbshipit-source-id: 8f42cca0be99022b038742728ad4fec6c04d5cf0
Summary:
This change removes the concept of `StateTarget`, replacing its role with `ShadowNodeFamily`.
When `StateTarget` was built, we didn't have a concept of `Family`, and when we added it we introduced a retain-cycle: ShadowNode -> Family -> StateTarget -> ShadowNode. This diff fixes that.
This change does not change conceptually how the state behaves, it just adjusts internal machinery.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19799013
fbshipit-source-id: c1360bfbf6b8ac34e2a856a40047eafeb50ed070
Summary:
Before this change, the concrete component-specific data/payload object associated with a State was stored inside a templated subclass as a normal instance variable; after the change, it's stored as a shared pointer inside the base class. The original motivation was that storing that inside subclass saves us one shared pointer and one heap allocation.
This approach overcomplicated a lot of things and all possible savings are probably compensated with additional complexity (we have to have templated state-update lamdas in subclasses and so on). And to update the data in the previous approach we need to create a shared pointer to data anyway.
This change will allow future improvements in the coming diff.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19799014
fbshipit-source-id: 287ed939353ba58d9e434d1502ecfbb208c6daa5
Summary:
Only use State Reconciliation during certain commits from ReactJS, not during State commits or others
The idea is that this will have better perf, since many types of commits don't need State Reconciliation at all.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19794999
fbshipit-source-id: 336325f225d993a6aae9f55cb95a7a2b38a6d853
Summary:
This implements proposal #2 in our State architecture doc: https://fb.quip.com/bm2EAVwL7jQ5
Problem description: see the text in the comment of TreeStateReconciliation.h
Solution: see also comments in TreeStateReconciliation.h.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D19617329
fbshipit-source-id: 845fb5fe27f2591be433b6d77799707b3516fb1a
Summary:
Systraces were disabled a year ago in D14019272 because we suspected they can negatively affect perf. We don't think this is the case anymore.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19786284
fbshipit-source-id: 185ed45b134fdcadf131cfddfcf8faf37537a684
Summary:
These props are applied to `View`, therefore they most not be a part of base text attributes, so we reset them.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D19764144
fbshipit-source-id: a0d2900e3161c47b83114360c843fa85a4389f8a
Summary:
Of course, compare_exchange_strong didn't actually do what I wanted.
Using a mutex is simpler and actually has the semantics we want: atomically get the current value, compare, and bail if the value is the same, or swap and continue.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19754451
fbshipit-source-id: 6b0aef217b235959af683ec5e31b07a0dd7bb040
Summary:
- We use a fork of Microsoft's react-native-windows which uses a fork of Facebook's react-native
- YGValue.h does not compile with Clang on Windows
- This change should fix that
- I want to put the change here so that it bubbles back to our fork > https://our.intern.facebook.com/intern/diff/D19656093/
#Changelog:
[General][Fixed] Make YGValue.h compile with Clang on Windows
Reviewed By: SidharthGuglani
Differential Revision: D19717489
fbshipit-source-id: ad867ecaf910bb64a777a06c656a1867bb15484b
Summary:
The documentation in two out of the three variants of `Function::callWithThis` incorrectly stated that the `this` value was to be `undefined` instead of set to `jsThis`, which contradicts the point of the `callWithThis` method existing in the first place. This diff fixes that issue.
Changelog: [General] [Fixed] - Fix documentation comments for HermesJS's `Function::callWithThis` method to accurately reflect how `this` is handled.
Reviewed By: jbower-fb
Differential Revision: D19714074
fbshipit-source-id: 123688c1f7e578d6356bec54604fb6e30f06b0b1
Summary:
Simple hack to prevent duplicate onLayout events from being emitted to JS.
Because of the addition of State Reconciliation to the Fabric lifecycle (see previous diff), in certain circumstances entire subtrees can be relayed-out many, many times even though they aren't changing. For JS product code that responds to every onLayout and forces a ReactJS tree commit (see: some usages of VirtualizedList) this can cause an async infinite loop of commits and layouts even though the tree and the LayoutMetrics aren't actually changing. Even though nothing is changing, it can still cause serious performance regressions and even some bugs since the internals of various state machines may assume onLayout won't be called many times.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19715280
fbshipit-source-id: d879e24f1c7b1f710ad430b7473aa9293d093dea
Summary:
Adding a `.clang-tidy` to a bunch of dirs under `react-native-github/ReactAndroid` and `react-native-github/ReactCommon`.
I don't want to add a single `.clang-tidy` at the root because we'll need more fine-grained control over what checks are enabled in different parts of the codebase; for example, fabric will and TM will probably have more checks enabled than older parts of the codebase that we're not actively modernizing, and the Hermes team probably wants its own config to be consistent with the rest of their codebase.
Starting off each `.clang-tidy` by only enabling clang-diagnostic; this is just to test that it's working. In the future, we'll work with the community to gradually enable more checks.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19705749
fbshipit-source-id: 979cea053b645ac4a9790340033bfcfb49ca0f97
Summary:
Changelog: [internal]
`ShadowNodeFamily` can be used as target in state updates instead of `ShadowNode`.
Reviewed By: shergin
Differential Revision: D19517428
fbshipit-source-id: 6831357e749239d5afec1dfd2d44a26ca6553e51
Summary:
Changelog: [internal]
Merges all of responsibilities of `StateCoordinator` into `ShadowNodeFamily`.
Reviewed By: shergin
Differential Revision: D19500104
fbshipit-source-id: f31ffded5a840e722fd898eef6a9f52cd2186df7
Summary:
This is a quite fragile and important part of the render engine. We need to dirty Yoga node only in cases where a change affects layout. In the case of over-dirtying, we can kill performance. In the case of under-dirtying, we can produce an incorrect layout.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596279
fbshipit-source-id: 9f2ac67c44cb35c8ba44be1025b94b7921b74e17
Summary:
This is aligned with all simular changes that we do for `*Props` types (see D19583582 and D19390813).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596281
fbshipit-source-id: 283b89c65504e20a461b12c2d1218325c6621dd8
Summary:
Reasons:
* The name of the method now better represent what it's doing;
* It exposes information about "dirty" state of the node without opening actual `LayoutableShadowNode` protected APIs;
* It's a tiny bit faster now because it checks the flag before calling Yoga.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596282
fbshipit-source-id: 3d87d9d5ba20bb8e360683f149b5ebf90beecd65
Summary:
After D19565499, the `LongLivedObjectCollection` will be cleared on the JS thread when the jsi::Runtime is deleted. This diff makes it so that we never hold strong references to `CallbackWrapper`s in our Android TurboModules infra. Therefore, we can leverage the changes in D19565499 to ensure that our `jsi::Function`s are deleted before the `jsi::Runtime`.
## Caveat
If you delete a TurboModule by itself, it's jsi::Functions that haven't been invoked won't be released. This is also the case for iOS. I plan to fix this for both iOS and Android at a later point in time.
Changelog:
[Android][Fixed] - Refactor jsi::Function cleanup in TurboModules
Reviewed By: mdvacca
Differential Revision: D19589151
fbshipit-source-id: efa3cc6c83634014159ac7500dcf6bef9c925762
Summary:
It makes perfect sense because `Builder` builds totally new shadow trees, so those are not sealed or used by anyone yet.
Assigning it to const shared pointer will do logical sealing (and it does not requires const-cast).
Fewer const-casts in the code, fewer bugs.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596284
fbshipit-source-id: 75d1c706034958ba7e4bc80a68af75a57c46eb6f
Summary:
Before this change `Element<X>` cannot have children of `Element<Y>` which was wrong because we don't have such limitation in Fabric.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596283
fbshipit-source-id: 9002f5dd42b3d05e7cf492499499399c97b58152
Summary:
Changelog: [Internal]
# Analysis
Measure returns following values for `frame.y` when tapping item in bottom sheet.
Fabric 412.33331298828125.
Paper 49.
In Paper, the frame.y returned is the position of tapped item in bottom sheet relative to the bottom sheet itself, which is correct.
This can happen in both BottomSheet and Modal.
# Why it happens?
In [UIManager.getRelativeLayoutMetrics](https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactCommon/fabric/uimanager/UIManager.cpp?commit=a372cf516ba1245ad9462e68376ee759c118a884&lines=172-181) if `ancestorShadowNode` is nullptr we populate `ancestorShadowNode` with `rootShadowNode` for the surface.
Problem is that BottomSheet that is presented, is not a separate surface. This means that we climb up the shadow node hierarchy all the way to root shadow node and keep adding offsets. Even though we should stop when we hit shadow node representing BottomSheet.
# How could be this fixed?
I think we should add a new shadow node trait that would mark node as root node. As we are traversing the shadow node tree upwards and adding offset of that node, once we hit a node that is "anchor", we would immediately stop the traversal.
This solution is inspired by Paper where the node representing BottomSheet has a special flag which marks it as "root" node.
accepttoship
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D19640454
fbshipit-source-id: bde623b1f41a9745a41f0aada7221bf924fad453
Summary:
## Description
You're not supposed to hold on to JSI objects (ex: `jsi::Function`) past the point where their `jsi::Runtime` is deleted. Otherwise, we get a dangling pointer crash, like this: T60262810! Historically, this cleanup problem has always been really tricky to get right. With this diff, I hope to fix that problem once and for all by deleting all `jsi::Function`s when we delete the global `__turboModuleProxy` function.
## Current Setup
- The TurboModules infra uses weak references to `CallbackWrapper`s to hold on to the `jsi::Function`s passed from JS to ObjC.
- The LongLivedObjectCollection holds on to strong references to `CallbackWrapper`s. This ensures that the `jsi::Function`s aren't deleted prematurely. This also means that we can use `LongLivedObjectCollection` to delete all `CallbackWrappers`.
- `TurboModuleBinding` is the abstraction we use to install the global `__turboModuleProxy` function. It is owned by `TurboModuleManager`, and `TurboModuleManager` uses it to clear all references to `jsi::Function`s, when we delete all NativeModules.
## Solution
1. Transfer ownership of `TurboModuleBinding` from `TurboModuleManager` to the `__turboModuleProxy` function.
2. Clear the `LongLivedObjectCollection` when `TurboModuleBinding` is deleted.
Changelog:
[iOS][Fixed] - Clear all held jsi::Functions when jsi::Runtime is deleted
Reviewed By: JoshuaGross
Differential Revision: D19565499
fbshipit-source-id: e3510ea04e72f6bda363a8fc3ee2be60303b70a6
Summary:
Changelog: [Internal]
Exposes `synchronouslyWaitForStage` to `RCTFabricSurface`.
This is a first step towards having screenshot tests rendered with Fabric.
Reviewed By: shergin
Differential Revision: D19603837
fbshipit-source-id: 26c14cf3bbd67fea96319ff08d3321557ddcdd9c
Rachel Nabors
Radek Pietruszewski
Janette Cheng
Samuel Susla
Moti Zilberman
Valentin Shergin
Haozhun Jin
David Detlefs
Jason Safaiyeh
Riley Dulin
Josh Leibsly
empyrical
Joshua Gross
Christoph Purrer
Pieter De Baets
Gabriel Nunes
Emily Janzer
Ramanpreet Nara
