Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43599
iOS E2E tests non-deterministic rendering happens around examples with a border set to `StyleSheet.hairlineWidth`. That value has special subpixel math, that doesn't seem to render consistently on iOS (this is its own bug).
To unblock adding some new E2E iOS TextInput tests, this removes usage of `hairlineWidth` in styles, and more generally, tries to unify TextInput styles in the examples.
This will break a whole bunch of RNTester Jest E2E baselines on different apps, which I will update from land-time runs or after continuous builds are available for different endpoints.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D55213090
fbshipit-source-id: 6d81b9355adc538a3ade6f50ef93c3ca08782ae7
Summary:
Stacked on top of #28498 for test fixes.
### Don't Rethrow
When we started React it was 1:1 setState calls a series of renders and
if they error, it errors where the setState was called. Simple. However,
then batching came and the error actually got thrown somewhere else.
With concurrent mode, it's not even possible to get setState itself to
throw anymore.
In fact, all APIs that can rethrow out of React are executed either at
the root of the scheduler or inside a DOM event handler.
If you throw inside a React.startTransition callback that's sync, then
that will bubble out of the startTransition but if you throw inside an
async callback or a useTransition we now need to handle it at the hook
site. So in 19 we need to make all React.startTransition swallow the
error (and report them to reportError).
The only one remaining that can throw is flushSync but it doesn't really
make sense for it to throw at the callsite neither because batching.
Just because something rendered in this flush doesn't mean it was
rendered due to what was just scheduled and doesn't mean that it should
abort any of the remaining code afterwards. setState is fire and forget.
It's send an instruction elsewhere, it's not part of the current
imperative code.
Error boundaries never rethrow. Since you should really always have
error boundaries, most of the time, it wouldn't rethrow anyway.
Rethrowing also actually currently drops errors on the floor since we
can only rethrow the first error, so to avoid that we'd need to call
reportError anyway. This happens in RN events.
The other issue with rethrowing is that it logs an extra console.error.
Since we're not sure that user code will actually log it anywhere we
still log it too just like we do with errors inside error boundaries
which leads all of these to log twice.
The goal of this PR is to never rethrow out of React instead, errors
outside of error boundaries get logged to reportError. Event system
errors too.
### Breaking Changes
The main thing this affects is testing where you want to inspect the
errors thrown. To make it easier to port, if you're inside `act` we
track the error into act in an aggregate error and then rethrow it at
the root of `act`. Unlike before though, if you flush synchronously
inside of act it'll still continue until the end of act before
rethrowing.
I expect most user code breakages would be to migrate from `flushSync`
to `act` if you assert on throwing.
However, in the React repo we also have `internalAct` and the
`waitForThrow` helpers. Since these have to use public production
implementations we track these using the global onerror or process
uncaughtException. Unlike regular act, includes both event handler
errors and onRecoverableError by default too. Not just render/commit
errors. So I had to account for that in our tests.
We restore logging an extra log for uncaught errors after the main log
with the component stack in it. We use `console.warn`. This is not yet
ignorable if you preventDefault to the main error event. To avoid
confusion if you don't end up logging the error to console I just added
`An error occurred`.
### Polyfill
All browsers we support really supports `reportError` but not all test
and server environments do, so I implemented a polyfill for browser and
node in `shared/reportGlobalError`. I don't love that this is included
in all builds and gets duplicated into isomorphic even though it's not
actually needed in production. Maybe in the future we can require a
polyfill for this.
### Follow Ups
In a follow up, I'll make caught vs uncaught error handling be
configurable too.
---------
DiffTrain build for commit https://github.com/facebook/react/commit/6786563f3cbbc9b16d5a8187207b5bd904386e53.
Changelog:
[Internal]
Reviewed By: kassens
Differential Revision: D55408481
Pulled By: yungsters
fbshipit-source-id: 598aa306369e21cb3e93ad6041a87bfbaa9eef9e
Co-authored-by: Ricky Hanlon <rickhanlonii@gmail.com>
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43686
Under Node 20, the use of `new Buffer(string)` is deprecated and logs a warning. This replaces it with the recommended `Buffer.from(string)`.
Changelog:
[General][Fixed] FIx "Buffer() is deprecated" warning from debugger proxy.
Reviewed By: huntie
Differential Revision: D55472025
fbshipit-source-id: 8b5af9e2d7e026cbdf6aa68f71ff0f856fb164db
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43698
changelog: [internal]
RCTViewComponentView was missing isAccessibilityElement override. Here I add it and use contentView to determine if element is accessible.
Reviewed By: rubennorte
Differential Revision: D55483944
fbshipit-source-id: f29c82a42d17140ae421f27871a3bf6f7f36bc12
Summary:
## Context
Prior, ReactContext used to implement bridge logic.
For bridgeless mode, we created BridgelessReactContext < ReactContext
## Problem
This could lead to failures: we could call bridge methods in bridgeless mode.
## Changes
Primary change:
- Make all the react instance methods inside ReactContext abstract.
Secondary changes: Implement react instance methods in concrete subclasses:
- **New:** BridgeReactContext: By delegating to CatalystInstance
- **New:** ThemedReactContext: By delegating to inner ReactContext
- **Unchanged:** BridgelessReactContext: By delegating to ReactHost
## Auxiliary changes
This fixes ThemedReactContext in bridgeless mode.
**Problem:** Prior, ThemedReactContext's react instance methods did not work in bridgeless mode: ThemedReactContext wasn't initialized in bridgeless mode, so all those methods had undefined behaviour.
**Solution:** ThemedReactContext now implements all react instance methods, by just forwarding to the initialized ReactContext it decorates (which has an instance).
Changelog: [Android][Removed] Delete ReactContext.initializeWithInstance(). ReactContext now no longer contains legacy react instance methods. Please use BridgeReactInstance instead.
Reviewed By: javache
Differential Revision: D53145010
fbshipit-source-id: 2405bc24afb00864117d3c504fc9c4cbffd7203a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43696
This just sets explicitApi to true for every module inside ReactAndroid
Changelog:
[Internal] [Changed] - Flip explicitApi to True for everyone
Reviewed By: tdn120
Differential Revision: D55478674
fbshipit-source-id: c9aeba89ad5b0f88bca7fd480c6aa66e0152a456
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43699
This diff initializes `RELEASE_VERSION` with the value that is provided by the `get_react_native_version` job (it stores its output into `/tmp/react-native-version`).
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D55484988
fbshipit-source-id: f0b5bb473096f3691f50152beb3181a454916fdc
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43661
# Changelog: [Internal]
1. Remove `BridgelessDebugReactPackage.java`, this was added in D43407534. Technically, its the same as `DebugCorePackage.java`.
2. `ReactInstance` to add `DebugCorePackage`, so `DebuggingOverlay` view manager will be included in the bridgeless build.
3. Fix `RNTesterApplication.kt` to NOT create `MyLegacyViewManager` for every possible viewManagerName, apart from `"RNTMyNativeView"`, return null instead.
Reviewed By: cortinico
Differential Revision: D55375350
fbshipit-source-id: 1d3cb6b5ad3c0248df1def9f37c8c49b308f4473
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43690
# Changelog: [Internal]
Fixes https://github.com/facebook/react-native/issues/43678.
The issue is that once `getInspectorDataForViewAtPoint` is imported, it should throw if RDT global hook was not injected. ReactDevTools overlay imports `getInspectorDataForViewAtPoint`, this is why it did throw in testing environment.
ReactDevToolsOverlay JSX-element is already gated with RDT global hook check, adding a deferred import, same as it was already implemented for Inspector.
Still unclear to me how this didn't throw all this time while using the Catalyst / RNTester.
Reviewed By: cortinico
Differential Revision: D55474774
fbshipit-source-id: 759e5e8227cc7534193e5b95616b6099c15f5cb5
Summary:
When RN moved Button component from a class component to a function component (https://github.com/facebook/react-native/commit/07e8ae42bed71f54bbe0e786ccad88b2f14648a4) a forwardRef call was not added to the Button control. This caused a set of tests downstream in React Native for Windows to fail because they rely on being able to pass a ref through to the Button control.
## Changelog:
[GENERAL] [FIXED] - Adds forwardRef call to new functional component implementation of Button control.
Pull Request resolved: https://github.com/facebook/react-native/pull/43666
Test Plan: Button render remains the same.
Reviewed By: fabriziocucci
Differential Revision: D55398765
Pulled By: zeyap
fbshipit-source-id: ba32c764c16cb529ab1c92cb7888f2bae0f16f5f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43664
Changelog: [internal]
Now that we have the event loop, we can modify the implementation of `MutationObserver` (which is still not enabled by default) to dispatch the notifications as microtasks, making the API more spec-compliant.
Reviewed By: javache
Differential Revision: D55380178
fbshipit-source-id: f876ffba49f9744f6603053f1485e7c2f43cb230
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43672
Changelog: [internal]
Small cleanup to move the last C++ native module defined in a JS directory to the new directory in `react-native/ReactCommon/react/nativemodule`.
Reviewed By: javache
Differential Revision: D55384106
fbshipit-source-id: 3bf477c2aceab6838f7f8131174b6eb74e890a23
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43663
Changelog: [internal]
We have a new directory for built-in C++ native modules, but the native modules for `MutationObserver` and `IntersectionObserver` were created before we had it.
This moves the native module for `MutationObserver` to `react-native/ReactCommon/react/nativemodule/mutationobserver` to follow the convention.
Reviewed By: javache
Differential Revision: D55380179
fbshipit-source-id: 0c64acbec973f2e5b57a0e38a0992bba49a01a45
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43653
Changelog: [internal]
This will allow us to clean up some code in `UIManager` (using methods in the native module instead) and prepare to use the DOM APIs in OSS behind a feature flag.
This doesn't enable the DOM APIs in OSS, only the native module.
Reviewed By: javache
Differential Revision: D55365252
fbshipit-source-id: 70ec0eb022df586ad554c5b8ce6915b8ceddef5f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43659
Changelog: [internal]
This adds an implementation for the legacy layout measurement methods in React Native (`measure`, `measureInWindow` and `measureLayout`) in the DOM native module, so we can clean up the API from the `nativeFabricUIManager` binding.
Reviewed By: javache
Differential Revision: D55368141
fbshipit-source-id: 196d4d29be3b78ffc22fdc136be6e0cf5ab9dd26
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43679
The current implementation of clear-with-predicate first copies unconsumed
elements and then all others. This works correctly when the buffer is full
(wraps around), but fails if size() < maxSize: add() may no longer insert
an element in the correct position (after the last unconsumed entry; see
new unit test).
Replace it with a loop that iterates over all entries in order, and adjusts
cursorStart and cursorEnd to point to the last numToConsume elements of the
vector.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D55273402
fbshipit-source-id: 647dc35faeb35c7fa99b8113cf85ce7f02f073e5
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43676
changelog: [internal]
calling super invalidates eventEmitter. EventEmitter should be invalidated before reseting contentOffset on `_scrollView. Otherwise, UIScrollView::setContentOffset is called and it calls delegate method: `scrollViewDidScroll`.
Reviewed By: javache
Differential Revision: D55375060
fbshipit-source-id: f697805eb1ca05d15cf498ff9e5e06e90eb7ac56
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43538
The Hermes RuntimeConfig for bridgeless accidentally force-disabled ES6Proxy, resulting in https://github.com/facebook/react-native/issues/43523
Let's remove the incorrect override.
To test using RNTester, add the following change:
```
diff --git a/packages/rn-tester/js/RNTesterAppShared.js b/packages/rn-tester/js/RNTesterAppShared.js
index 87cb6b69dfe..f2512d09c5a 100644
--- a/packages/rn-tester/js/RNTesterAppShared.js
+++ b/packages/rn-tester/js/RNTesterAppShared.js
@@ -50,6 +50,8 @@ const RNTesterApp = ({
);
const colorScheme = useColorScheme();
+ new Proxy({}, {});
+
const {
activeModuleKey,
activeModuleTitle,
```
Before this change, RNTester will get an error at start-up. After, the app loads correctly.
Changelog: [General][Fixed] Correctly keep ES6Proxy for bridgeless mode
Reviewed By: cortinico
Differential Revision: D55045780
fbshipit-source-id: 666b99712d35622f87d42f22a4611851df67d905
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43251
Integrates the modern CDP backend with `CatalystInstanceImpl` (the React Native instance implementation) on Android.
This complete the modern CDP integration for Bridge.
Changelog: [Internal]
Reviewed By: motiz88
Differential Revision: D51458010
fbshipit-source-id: 6f73868da9d0d4cc5d086a4569c78444cb1b83ec
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43657
This include is not correct and is breaking the OSS build
Changelog:
[Internal] [Changed] - Fix header import inside ReactInstanceManagerInspectorTarget.h
Reviewed By: rshest
Differential Revision: D55368321
fbshipit-source-id: 0530257ad5c548476beb882174d73842775b3726
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43646
PerformanceEntryReporter maintains a buffer of RawPerformanceEntry objects,
as well as a set of _pointers_ to elements within that buffer, used to find
entries by name.
However, those pointers aren't stable: BoundedConsumableBuffer internally uses
a vector, and there are a few cases where existing references get invalidated:
- When the vector's capacity changes (as new entries get inserted) [1]
- After calling clear-with-predicate, which copies elements into a new vector
This causes nameLookup to contain dangling pointers, and subsequent operations
on it can result in use-after-free.
Fix this by having BoundedConsumableBuffer reserve space for maxSize entries
up front (which ensures that existing pointers remain valid after adding new
elements) and by rebuilding nameLookup after clearing entries by name.
Note that reserve() causes the buffer's memory use to be higher than before in
case where the number of elements is small relative to the max size. Given the
(only) existing usage in PerformanceEntryReporter, as well as the property that
consumed elements remain in the buffer, that cost should be minor.
Changelog: [Internal]
[1] https://en.cppreference.com/w/cpp/container/vector/push_back
Reviewed By: rshest
Differential Revision: D55273403
fbshipit-source-id: c8f33203ae32685e29afa7f8e33edf1284d66e0f
Summary:
Small typo I encountered while trying to build custom C++ type converters :)
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[INTERNAL] [FIXED] Fixed a small typo in the "unsupported type" error message
Pull Request resolved: https://github.com/facebook/react-native/pull/43650
Reviewed By: zeyap
Differential Revision: D55364068
Pulled By: cortinico
fbshipit-source-id: 5a1bc9443c82f2473860f379c9ae063cd6e3ceb4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43655
## Changelog:
[Internal] -
The corresponding prop `ScrollBar.persistentScrollIndicator` was only passed to the Android platform code an `ScrollBar.horizontal` wasn't passed to native at all.
On other platforms we need those props to be available on the C++ side, so this exposes them to the corresponding C++ ScrollViewProps.
Reviewed By: sammy-SC
Differential Revision: D55367445
fbshipit-source-id: e8abca3a2b56a8e7c03593a6c4297f90749ac8fd
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43652
This improves the signature of our existing DOM APIs in 2 ways:
1. It replaces the use of `tuples` in `DOM.{h,cpp}` with safer structs.
2. It removes some unnecessary optionals from the API, returning the default values from the C++ API directly when appropriate.
It still preserves the use of tuples in the native module because objects are not properly supported in the codegen.
Changelog: [internal]
Reviewed By: NickGerleman
Differential Revision: D55316654
fbshipit-source-id: 16ce5ef62ca427cdcd6b9757d77db040e0ccc8b1
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43626
The things that create ReactApplicationContext should instead create BridgeReactContext.
Long-term, ReactApplicationContext will be abstract. This diff pulls noise out from that eventual diff.
Changelog: [Internal]
Reviewed By: arushikesarwani94
Differential Revision: D55218591
fbshipit-source-id: d359c794f3da4a1ecb2fa8edbed5eeeb620b137b
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43627
Eventaully, ReactApplicationContext.initailizeWithInstance() will be moved to BridgeReactContext.
Doing the migration up-front to remove noise from the eventual diff.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D55218593
fbshipit-source-id: c542c44cf8b36b9dc2a01db2bf6173639fb8698a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43624
Eventually, we will move all bridge methods to BridgeReactContext.
Long-term, ReactApplicationContext and ReactContext will become abstract. And these two contexts will only contain methods common to both modes.
Changelog: [Android][Added] Introduce BridgeReactContext
Reviewed By: cortinico
Differential Revision: D55218592
fbshipit-source-id: 731d4940d492a1ed3f855c9a181d2a6f9eeb9623
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43250
Integrates the modern CDP backend with `ReactInstanceManager` on Android.
`ReactInstanceManager` is equivalent to the CDP page / `HostTarget` concept, therefore we register the `addPage`/`removePage` calls with this object's lifecycle.
Implementation notes:
- `ReactInstanceManagerInspectorTarget` is created to avoid converting `ReactInstanceManager` to JNI (impacting tests).
- Its constructor receives a `TargetDelegate` object, so that we avoid passing the entire `ReactInstanceManager` class through (avoids cyclic dependency from `com.facebook.react.bridge` to `com.facebook.react`).
Changelog:
[Internal] - Register `ReactInstanceManager` with modern CDP backend
Reviewed By: motiz88
Differential Revision: D51456960
fbshipit-source-id: 942255bb2487fdc581eb4fa0903c8e68106f8b35
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43615
This class is used for a one-time setup of bindings into the jsi::Runtime. The lambda will retain ownership of any java components required, and we can rely on the teardown of the JS runtime to clean up any dependencies.
Changelog: [Internal]
Reviewed By: dmytrorykun
Differential Revision: D55241233
fbshipit-source-id: f7541f28277307be9b3a5f4f780c7eca1a467c57
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43614
Storing a global_ref to `jThis` in a JNI hybrid object leads to a reference cycle which cannot be cleaned up by Java GC. The Java object will keep the C++ object alive and vice versa.
In this class, we didn't seem to need this reference anyway.
Changelog: [Internal]
Reviewed By: dmytrorykun
Differential Revision: D55241232
fbshipit-source-id: 933521ed1483c149f693988ca17f4c53565dbfe0
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43612
This class is used for a one-time setup of bindings into the jsi::Runtime. The lambda will retain ownership of any java components required, and we can rely on the teardown of the JS runtime to clean up any dependencies.
Changelog: [Internal]
Reviewed By: dmytrorykun
Differential Revision: D55241234
fbshipit-source-id: ee80ca0f91ebe95f8c44874b27912b260f70b572
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43618
Both common and Android implementations of AsyncEventBeat use weak_ptrs
to determine if the object is still valid before invoking the callback.
Move the write to `isBeatCallbackScheduled_` down so it's protected by
that same check.
Changelog: [Internal]
Reviewed By: javache, NickGerleman
Differential Revision: D55226529
fbshipit-source-id: 9e2a34369346d11dcea69d120dfa5935320f9ba1
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43639
Currently, the `react-native/dev-middleware` inspector proxy intercepts `Debugger.scriptParsed` notifications from the target and replaces `sourceMapURL` with a data uri, via an async fetch from Metro. During this async fetch, other notifications from the debugger may pass through the proxy, which results in the frontend receiving them before `Debugger.scriptParsed`.
This reordering causes problems in breakpoint resolution and pausing, because `Debugger.breakpointResolved` and `Debugger.paused` events may reference `scriptId`s unknown to the frontend while the corresponding `Debugger.scriptParsed` is delayed.
In particular, breakpoint UI state and backend state can fall out of sync, and breakpoints hit may open to the incorrect source location.
This diff modifies the proxy to use a simple per-target promise queue to ensure messages are handled in the order they were received from the target.
Changelog:
[General][Fixed] Fix breakpoints opening to incorrect location or disappearing from debugger frontend UI.
Reviewed By: motiz88
Differential Revision: D55200617
fbshipit-source-id: 27c95f822266875ed668d0bf8a525da49554cafd
Alex Taylor (alta)
Nick Gerleman
sebmarkbageandRicky Hanlon
Tim Yung
TatianaKapos
Viresh Umbre
Fabrizio Bertoglio
Rob Hogan
Samuel Susla
Ramanpreet Nara
Nicola Corti
Dmitry Rykun
Edmond Chui
Ruslan Lesiutin
Chiara Mooney
Rubén Norte
szymonrybczak
Arushi Kesarwani
Tomislav Novak
zhongwuzw
Pieter De Baets
Kevin Gozali
Alex Hunt
Sam Zhou
Evert Eti
Marc Rousavy
Ruslan Shestopalyuk