mirror of
https://github.com/react/react-native.git
synced 2026-09-28 05:13:06 +08:00
main
1626
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Peter Abbondanzo
|
88b9c3fb7c |
Retry suite-level runtime failures (#58631)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58631 Fantom's CI retries use Jest's `--onlyFailures` mode. Jest 29's default sequencer caches suite-level runtime errors as passing because they contain no failed test cases, so retries select no suites. See https://github.com/react/react-native/actions/runs/35630015753/job/106440689782?pr=58624 of an example of this, where retries didn't actually do anything Add a Fantom-specific sequencer that marks runtime-error suites as failed only for cache bookkeeping. This preserves the original test reporting while ensuring targeted retries rerun the crashed suite. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D121063351 fbshipit-source-id: e24737cc247ea0e11aa04dac74d557a3f41f9db6 |
||
|
Peter Abbondanzo
|
4ffc4a6523 |
Move sample TurboModule spec to RNTester (#58626)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58626 Move `NativeSampleTurboModule` into RNTester so React Native core codegen no longer treats the sample as public API. Retarget the sample native implementations to RNTester's generated `AppSpecs` on Android, iOS, and macOS. Changelog: [General][Removed] - Remove the sample TurboModule from React Native's public API Reviewed By: christophpurrer, javache Differential Revision: D121032097 fbshipit-source-id: 865222ce4d4d4c8da323ae444eb43bec940ef000 |
||
|
Jakub Piasecki
|
900b717e83 |
Update branching implementation to default to the JS thread merge (#58615)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58615 Changelog: [Internal] The current behavior, merge on the main thread, is kept behind the new `enableFabricCommitBranchingMergeOnMainThread` flag. Reviewed By: rubennorte Differential Revision: D120981616 fbshipit-source-id: 47f00ddc150f154481cc908548f39662abf32cbf |
||
|
Janic Duplessis
|
b439ef7c3f |
Process synchronous event beats in the frame that requested them (#58530)
Summary: `EventEmitter::experimental_flushSync` only *requests* an event beat; the beat is processed at the next `EventBeat::induce`. On iOS the run loop observer that induces the beat runs before Core Animation's commit observer, so a request made from `layoutSubviews` — inside CA's commit cycle — is only processed one frame later. Anything that reports layout-driven state to JS synchronously (`VirtualView` mode changes, and safe area insets in the PRs that build on this) renders a frame late in exactly the cases that matter. `AppleEventBeat` now also schedules an induce in the **display phase of the current commit cycle**. Core Animation runs a commit as layout → display → commit, so a zero-sized layer marked as needing display during layout gets its `display` call after the whole layout pass and before the transaction is committed. That layer needs to live in the tree being committed, so the beat has to know which tree that is — and the emitter tells it: - `experimental_flushSync` carries the **tag of the emitting view** through `EventDispatcher` and `EventQueue` to `EventBeat::requestSynchronous(Tag)`, with `kNoTag` (https://github.com/react/react-native/issues/58531) meaning no view attribution; a no-argument overload keeps unattributed requesters and the existing tests unchanged. The emitter reads the tag from its `ShadowNodeFamily` at flush time; `kNoTag` if the family is already gone. - `AppleEventBeat` resolves the tag to the layer of the view's **window** through a resolver injected by `RCTSurfacePresenter` (`findComponentViewWithTag:` on the mounting registry — nullable, non-creating, main thread) and attaches its flusher layer there. The requesting view's window is by definition the root of the layer tree whose layout emitted the request, so the flusher is guaranteed a display phase in the current commit cycle — including for content UIKit mounts in a window of its own, like a full screen modal or LogBox. Requests within one cycle coalesce into a single induce. One related fix in `EventBeat` itself: a synchronous request is no longer stranded behind an already-scheduled asynchronous beat (it would silently lose its this-frame guarantee, and the leftover flag would make an unrelated later beat blocking). `AppleEventBeat.cpp` becomes `.mm` for the Objective-C. **Risk:** this changes when queued events are flushed on iOS for every `experimental_flushSync` caller — today `VirtualView`, and safe area insets with the PRs on top. The worst case is a beat processed a frame *earlier* than before, inside a Core Animation commit; the run loop observer path is untouched and still catches anything the display phase misses (an emitter with no tag, an unmounted view, a request off the main thread). Android ignores the tag. Revert is self-contained. ## Design Q&A: **What happens when two views in different windows update at once?** Each requesting window gets its own dirty flusher layer (the map is keyed by host layer), and the first `display` to fire induces the beat, which drains the whole event queue — every window's updates mount before that commit presents. The remaining flushers hit the `isEventBeatRequested_` guard and no-op, so it is one beat total, not one per window. If windows ever commit in separate transactions, each request still resolves within its own window's cycle, since its layer sits in the tree that emitted it. Only requesting windows carry a dirty layer. **Can the tag point at the wrong view — after an unmount, or a recycled view?** No. The tag comes from the emitter's `ShadowNodeFamily`, and a family keeps one tag for its whole life, across clones and state updates; if the family is already gone the flush carries `kNoTag` and skips the resolver. What changes over a view's life — its window — is read live: the tag resolves to a view at flush time and `view.window.layer` is looked up then, so a view that moved between windows targets its current tree. A view mid-unmount or recycled resolves to nil (the registry erases the entry and recycled views get tag `0`) and degrades to run-loop-observer timing. The tag only ever influences *where the induce is scheduled*, never what is delivered or to whom, so the blast radius of any staleness is one frame of timing, not correctness. **Does `VirtualView` need changes to benefit?** No — its sync mode-change flush goes through its own emitter, so the tag attribution is automatic. The case this improves is a mode change emitted during Core Animation layout (a resize pulling a virtualized item into view): on `main` that renders one frame late; here the induce lands in the display phase of the VirtualView's own window, including inside a full screen modal. ## Changelog: [INTERNAL] - Process synchronous event beats in the frame that requested them on iOS, scheduling the induce on the requesting view's window Pull Request resolved: https://github.com/react/react-native/pull/58530 Test Plan: New unit tests in `EventBeatTest.cpp` cover the beat semantics: a synchronous request during an already-scheduled asynchronous beat, coalescing, and induce ordering. They drive the protected `induce` through a subclass standing in for the platform. On device, with the safe area insets prop from the PRs above merged on top: an RNTester example renders a loud marker (yellow background) while a view observes the safe area but has not received an inset event yet, so any presented marker frame means the dispatch was not synchronous. The full apply → landscape → portrait sequence **inside a full screen modal** on an iPhone 17 Pro simulator, decomposed with ffmpeg into 982 frames and every frame scanned for the marker color — **zero marker frames**, and mid-rotation frames already carry the incoming orientation's insets, so the padding animates with the rotation. Scoped honestly: the first inset event after setting the prop is processed at the call site, so the marker primarily proves no regression; the same-frame path for layout-driven changes rests on the by-construction argument above plus the rotation frames. https://github.com/user-attachments/assets/0f2db837-c9c0-4457-96c2-847b7aecf10e `yarn fantom .../ViewSafeAreaInsets-itest.js` passes 4/4 with the prop merged on top. C++ API snapshots regenerated (`scripts/cxx-api/parser`, Doxygen 1.16.1): the deltas are the `requestSynchronous` overload pair, `EventEmitter::getTag`, the resolver type, and the `AppleEventBeat` constructor and destructor. --- **Stack** — split out of https://github.com/react/react-native/issues/57967, which stays open as the prototype and design discussion. GitHub will not take a fork branch as a pull request base, so each of these targets `main` and its diff contains the ones below it until they merge. Each PR is one commit on top of the previous one. This is the bottom of the stack, so its diff is already just this change. 👉 1. https://github.com/react/react-native/issues/58530 — Process synchronous event beats in the frame that requested them 2. https://github.com/react/react-native/issues/58109 — Add an `experimental_onSafeAreaInsetsChange` view prop 3. https://github.com/react/react-native/issues/58110 — Report the window safe area insets through Dimensions 4. https://github.com/react/react-native/issues/58112 — Render the internal SafeAreaView from the safe area insets prop 5. https://github.com/react/react-native/issues/58113 — Remove the native SafeAreaView and the deprecated public export An earlier variant that targeted the surface's root view instead of the view's window was closed in https://github.com/react/react-native/issues/58528; its review thread carries the analysis behind the window-based resolution. https://github.com/react/react-native/issues/58108 was the per-window predecessor this supersedes. Reviewed By: javache Differential Revision: D120200496 Pulled By: Abbondanzo fbshipit-source-id: b06ecb30935837abd6561f54674afef0cdf215a8 |
||
|
Calvin Liu
|
af4d8eb08a |
Preserve unset accessibilityState.selected in C++ props (#58599)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58599 `AccessibilityState::selected` was a plain `bool` defaulting to `false`, so the native side could not tell a component that is selectable but currently unselected (`accessibilityState={{selected: false}}`) from one that is not selectable at all (`accessibilityState={{}}`). Both arrived as `false`. The JS type is already `selected?: ?boolean`, so this is the bridge discarding a value the public API accepts. Make `selected` a `std::optional<bool>` defaulting to `std::nullopt`. JS accessibilityState native selected (before -> after) {} false -> undefined {selected: false} false -> false {selected: true} true -> true This aligns the representation with ARIA, which `accessibilityState` mirrors: the bool / tri-state split now tracks which ARIA attributes admit an undefined value. field ARIA value type admits undefined representation disabled boolean no bool busy boolean no bool selected boolean yes std::optional<bool> (changed) expanded boolean yes std::optional<bool> checked tristate yes CheckedState (None = unset) That is also why `disabled` and `busy` stay plain `bool`: ARIA gives them no undefined value, so there is no unset state to preserve. `expanded` was made optional for this same reason in https://github.com/facebook/react-native/pull/40881 and `checked` has always carried a `None`; `selected` was the outlier. Nor is "unset" merely "absent" for this attribute. `testing-library/dom` computes it as `boolean | undefined`, documented "false/true if (not)selected, undefined if not selectable" -- the same shape, with the same meaning, that this change introduces. Host platforms need the distinction: on Windows a selectable component must implement ISelectionItemProvider so UIA can report selection state, and with the old representation every component carrying an accessibilityState looked selectable. iOS and Android rendering is unchanged. Trait derivation coalesces the optional with `value_or(false)`, and the Android serializer omits the key when the value is unset, which `BaseViewManager#setViewState` already handles by falling back to `setSelected(false)`. Reviewer note: `std::optional<bool>` is contextually convertible to `bool`, so a bare `if (state.selected)` still compiles but tests engagement rather than value, silently marking an explicitly unselected component as selected. There is a regression test for that specific hazard. Fixes https://github.com/facebook/react-native/issues/46988 Supersedes https://github.com/facebook/react-native/pull/47296, which went stale. Changelog: [General][Breaking] - `AccessibilityState::selected` is now `std::optional<bool>` in C++ props, preserving an unset `selected` instead of coercing it to `false` Reviewed By: javache Differential Revision: D120049025 fbshipit-source-id: 53ac440976f00daffb3f0599c8841ada2a1452a6 |
||
|
Christoph Purrer
|
562fe3433a |
Remove Fabric and TurboModule dead config (#58434)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58434 Follow-up to D116318829, addressing rubennorte's review comment. Fabric and TurboModules shipped before bridgeless and are always on, so the toggles for them were hardcoded and read nowhere. Android, `DefaultNewArchitectureEntryPoint` — now only selects the release channel and loads the SO: - removed `fabricEnabled`, `turboModulesEnabled`, `concurrentReactEnabled` - removed the deprecated `load(turboModulesEnabled)` and `load(turboModulesEnabled, fabricEnabled)` overloads - removed `isConfigurationValid`, and with it `DefaultNewArchitectureEntryPointTest` (every test targeted it) - updated the 8 in-repo call sites that passed `fabricEnabled` into the deprecated 3-arg `DefaultReactActivityDelegate` constructor, which discarded it iOS: - removed `fabricEnabled` / `turboModuleEnabled` from `RCTRootViewFactoryConfiguration` - removed the corresponding `RCTDefaultReactNativeFactoryDelegate` stubs and the `RCTAppDelegate.h` doc references `ReactAndroid.api` and the `ReactApple*Cxx.api` snapshots are regenerated. One call site is not updated here: `users/zh/zhaogang/benchmarks/SimpleRN/android/app/src/main/java/com/simplern/MainActivity.kt` still imports `DefaultNewArchitectureEntryPoint.fabricEnabled`. It is a personal benchmark app under `users/` that is not materialized in this working copy, so it could not be edited. Changelog: [General][Breaking] - Remove the `fabricEnabled` / `turboModulesEnabled` / `concurrentReactEnabled` accessors and remaining deprecated `load` overloads from `DefaultNewArchitectureEntryPoint`, and the `fabricEnabled` / `turboModuleEnabled` properties from `RCTRootViewFactoryConfiguration`; Fabric and TurboModules are always enabled https://www.internalfb.com/agent-home?session_id=dmh-2bfbb113-43fc-4bc4-819d-874c5101a1c8 Reviewed By: javache Differential Revision: D119380472 fbshipit-source-id: 19c130c3dda5c1cd6d541f4e726d8041b515c05c |
||
|
Conner Reimers
|
7e02fc7e68 |
Remove the unused maximumFontSize paragraph attribute (#58529)
Summary: Fabric's `ParagraphAttributes` carries a `maximumFontSize` field, but it is not exposed by the `<Text>` or `<TextInput>` APIs, so normal JS usage leaves it as `NaN`. On iOS, `RCTTextLayoutManager` falls back to a 96pt maximum when it is `NaN`. On Android, the C++ side serializes it into MapBuffer key 7, but `TextLayoutManager` never reads that key. This PR removes the attribute: the `ParagraphAttributes` field, its raw-prop parsing in `conversions.h`, the paragraph and text input prop entries, and MapBuffer key 7 on both the C++ and Kotlin sides. - **iOS**: `RCTTextLayoutManager` now passes the largest font size in the attributed string to `scaleFontSizeToFitSize:` as the maximum instead of 96pt. `scaleFontSizeToFitSize:` returns early when the text already fits; otherwise `scaleFontSizeWithRatio:` applies `MIN(pointSize * ratio, maximumFontSize)` on each pass of a bisection over the ratio. With 96pt, fonts above 96pt were clamped to 96pt on every pass, and when that clamp alone made the text fit, the search went on to ratios above 1.0 and grew the remaining fonts up to 96pt. With the largest font size as the maximum the clamp is a no-op, the ratio-1.0 pass reproduces the original text, which is known not to fit, and every later pass shrinks. Fonts above 96pt are now scaled proportionally rather than capped, and text is never grown. Text at 96pt or smaller is unaffected. - **Android**: `PA_KEY_MAXIMUM_FONT_SIZE` is removed. Nothing reads it, so there is no behavior change. Split out of https://github.com/react/react-native/issues/58492. https://github.com/react/react-native/issues/58492 builds on this and reuses the largest-font-size helper for `minimumFontScale`. ## Changelog: [GENERAL] [REMOVED] - Remove the unused `maximumFontSize` paragraph attribute Pull Request resolved: https://github.com/react/react-native/pull/58529 Test Plan: **Unit tests** - Updated `ParagraphAttributesTest.cpp` for the removed field. - Regenerated the C++ API snapshots (`yarn cxx-api-build`). ```sh ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests 'com.facebook.react.views.text.*' ``` All 38 `com.facebook.react.views.text` tests pass. Reviewed By: cipolleschi Differential Revision: D120127100 Pulled By: javache fbshipit-source-id: 44e7de2c21c95500e721d5c6f4b4e207da369713 |
||
|
Janic Duplessis
|
a9473b5ed5 |
Name the no-tag sentinel and use it for the hardcoded -1 tags (#58531)
Summary: React tags are positive; `-1` marks the absence of one in several places with a bare literal. This names the convention — `kNoTag` in `ReactPrimitives.h`, next to the `Tag` alias and mirroring `ViewUtil.NO_SURFACE_ID` on the Android side — and replaces the literals across the renderer: `ShadowViewMutation`'s `parentTag` default, factories and comparison, the stub view tree (folding its duplicate `NO_VIEW_TAG` constant into `kNoTag`), the `CppMountItem` mirrors on Android, `UIManagerViewTransitionDelegate`'s `nativeTag` and the view transition fallback that feeds it, and the pointer-events no-override comparisons. Split out of the safe area insets stack: https://github.com/react/react-native/issues/58530 threads a `Tag` with no-tag semantics through `experimental_flushSync` and wants the named sentinel, but the cleanup stands on its own and can land regardless of that PR's fate. ## Changelog: [INTERNAL] - Name the no-tag sentinel (`kNoTag`) and use it for the hardcoded `-1` tags Pull Request resolved: https://github.com/react/react-native/pull/58531 Test Plan: Compiles via the Fantom tester build; `yarn fantom` mounting-heavy suite passes 224/224 (exercises the `StubViewTree` asserts). C++ API snapshots regenerated — the only delta is the new `kNoTag` constant. Reviewed By: javache Differential Revision: D120199347 Pulled By: Abbondanzo fbshipit-source-id: 4198a33fa601edff53c54bce935814b3414c5820 |
||
|
Nicola Corti
|
30b4c7e947 |
Add Swift to yarn format (#58463)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58463 Add `yarn format-swift` and `yarn format-check-swift` using Apple swift-format, and compose them into the repository-wide commands. The wrapper selects a repository-provided formatter when available or uses swift-format 6.3 or newer from the Swift toolchain. Missing tools produce environment-specific setup guidance before Swift is skipped. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D119487615 fbshipit-source-id: 4da9939aa7ddf400e810cd17aeec12aa6118023b |
||
|
Nicola Corti
|
2334bf755d |
Add Python to yarn format (#58465)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58465 Add `yarn format-python` and `yarn format-check-python` using Ruff, and compose them into the repository-wide commands. The wrapper selects a repository-provided Ruff binary when available or bootstraps pinned Ruff through Python 3 and pip. Missing tools produce environment-specific setup guidance before Python is skipped. Changelog: [Internal] Reviewed By: javache Differential Revision: D119487614 fbshipit-source-id: 4801ae3e565fcaa86a860cc5b8bb3b2d823d48df |
||
|
Nicola Corti
|
c9c293d061 |
Add Java to yarn format (#58466)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58466 Add `yarn format-java` and `yarn format-check-java` using the npm `google-java-format` package, which bundles google-java-format 1.23.0. This removes the Gradle build dependency and adds the offline package mirror and workspace lock entries. The wrapper discovers a suitable JDK when available and otherwise prints environment-specific Java 17 setup guidance before skipping Java. Changelog: [Internal] Reviewed By: javache Differential Revision: D119487613 fbshipit-source-id: e5ed299cc19f64bd5aceefd90c6160852be8ba37 |
||
|
Nicola Corti
|
07e4bf34d0 |
Add Kotlin to yarn format (#58462)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58462 Add `yarn format-kotlin` and `yarn format-check-kotlin` using the npm `ktfmt` package and its bundled formatter jar. This adds the offline package mirror and workspace lock entry without changing existing Kotlin source formatting or Gradle configuration. The wrapper discovers a suitable JDK when available and otherwise prints environment-specific Java 17 setup guidance before skipping Kotlin. allow-large-files: The npm package intentionally contains the upstream ktfmt executable jar so offline and public installs use the same formatter. Changelog: [Internal] Reviewed By: javache Differential Revision: D119487612 fbshipit-source-id: cb699a1286cffbf461ef69ed66f694e3f69415d0 |
||
|
Nicola Corti
|
ec9952a62d |
Check all C-family sources with clang-format (#58461)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58461 Add npm-driven clang-format commands for exported C-family sources. Expose `yarn format-cpp` and `yarn format-check-cpp`, and compose them into the repository-wide commands. The wrapper automatically selects the repository-provided formatter for its environment. Changelog: [Internal] Reviewed By: javache Differential Revision: D119487611 fbshipit-source-id: 9d2e4267ce0d80f2002c0d3648e62e53bd3284af |
||
|
Christoph Purrer
|
46662327fb |
Delete legacy RCTNativeAnimatedModule from RN iOS (#57904)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57904 The Old Architecture has been deleted from React Native iOS, which makes the legacy `RCTNativeAnimatedModule` unreachable: - Its JS spec only requests `'NativeAnimatedModule'` when `shouldUseTurboAnimatedModule()` is false, but `ReactInstance.cpp` sets `RN$Bridgeless = true` unconditionally on iOS, so that branch never runs. - Its only entry points were `-setBridge:` and `RCTUIManagerObserver`, and `RCTBridge` is now an all-nil stub with `RCTCxxBridge` deleted. - Nothing referenced it by filename except a single `legacy = True` Buck plugin provider. iOS Animated is served by `RCTNativeAnimatedTurboModule` (bridgeless ObjC) and by the C++ `facebook::react::AnimatedModule` behind `cxxNativeAnimatedEnabled()`. With the last bridge-dependent Animated consumer gone, also stripped the dead bridge plumbing it was the only reason for: - `RCTNativeAnimatedNodesManager -initWithBridge:surfacePresenter:` loses `bridge:`. - `RCTPropsAnimatedNode -connectToView:viewName:bridge:surfacePresenter:` loses `bridge:` and the `RCTUIManager` fallback in `-updateView`, which collapses to the single `synchronouslyUpdateViewOnUIThread:props:` call. - `viewName:` goes too, since it was only used to look up a legacy view manager. The C++ (`connectAnimatedNodeToView(Tag, Tag)`) and Kotlin (`connectAnimatedNodeToView(animatedNodeTag, viewTag)`) nodes managers already take two arguments, and the sole remaining ObjC caller passed `nil`. `RCTNativeAnimatedTurboModule` is deliberately NOT renamed back to `RCTNativeAnimatedModule` — that is a much wider rename, best done separately. Changelog: [iOS][BREAKING] Delete legacy RCTNativeAnimatedModule from RN iOS Reviewed By: zeyap Differential Revision: D115663619 fbshipit-source-id: d9c703171c2121e8456d923e4db58e624c9040f5 |
||
|
Christoph Purrer
|
2a9eca81b8 |
Remove bridgelessEnabled from RCTRootViewFactoryConfiguration (#58416)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58416 Follow-up to the Android change in the previous diff, applying the same cleanup to iOS. Bridgeless is the only supported mode, so the `bridgelessEnabled` surface on `RCTRootViewFactoryConfiguration` was already deprecated and hardcoded: the property was assigned `YES` in every initializer, the two deprecated initializers ignored the argument entirely, and `RCTDefaultReactNativeFactoryDelegate` returned `YES` unconditionally. Nothing read the value. Changes: - Removed the `bridgelessEnabled` property from `RCTRootViewFactoryConfiguration`. - Removed the two deprecated initializers `initWithBundleURLBlock:newArchEnabled:turboModuleEnabled:bridgelessEnabled:` and `initWithBundleURL:newArchEnabled:turboModuleEnabled:bridgelessEnabled:`. Both were marked `__deprecated` and discarded all arguments except the bundle URL, delegating to the designated initializer. - Removed the `bridgelessEnabled` method from `RCTDefaultReactNativeFactoryDelegate`. It was not declared in any header or protocol. - Updated the one caller in `RCTReactNativeFactory` to use `initWithBundleURLBlock:newArchEnabled:`. Behavior is unchanged: bridgeless remains unconditionally enabled. Callers of the designated `initWithBundleURLBlock:newArchEnabled:` and `initWithBundleURL:newArchEnabled:` initializers are unaffected. Changelog: [iOS][Breaking] - Remove the deprecated `bridgelessEnabled` property and the deprecated `initWithBundleURLBlock:newArchEnabled:turboModuleEnabled:bridgelessEnabled:` / `initWithBundleURL:newArchEnabled:turboModuleEnabled:bridgelessEnabled:` initializers from `RCTRootViewFactoryConfiguration`; bridgeless is always enabled Reviewed By: rubennorte Differential Revision: D116318829 fbshipit-source-id: 9a8c232f22a9435934f848a7a80401841c2e29a2 |
||
|
Christoph Purrer
|
0e01003f53 |
Remove legacy RCTTimingModule (#57037)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57037 ## Changelog: [IOS][Fixed] Remove legacy RCTTimingModule Reviewed By: javache Differential Revision: D107201906 fbshipit-source-id: 34e006cd046795c80bd234c13979110aadfa9f58 |
||
|
Edward Kimmel
|
5646c31dff |
fix: rootMargin ignored when IntersectionObserver root clips (#58177)
Summary: Hit this using IntersectionObserver on a ScrollView with `rootMargin` to preload content below the fold. `rootBounds` grew by the margin, but `intersectionRatio` stayed 0 for anything past the clip. We're clipping the target to the root's overflow (the ScrollView / `overflow: hidden` box) *before* intersecting with the margin-expanded rect. That's not what the spec does. [Compute the intersection](https://w3c.github.io/IntersectionObserver/#calculate-intersection-rect-algo): walk containing blocks *while container is not root* and clip there, then intersect with the [root intersection rectangle](https://w3c.github.io/IntersectionObserver/#root-intersection-rectangle), which already includes `rootMargin`. The spec also calls out that `rootMargin` only applies to the root — clipping by any other ancestor is unchanged. This stops overflow-clipping the observation root itself. Intermediate clippers stay as they are, so a viewport-rooted observer still can't see past a ScrollView. That's `scrollMargin`, which we don't implement. -- Lots of words to say: This enables rootMargin to correctly extend the viewport of ScrollViews or any other view with overflow clipping when using that view as the root. This also gives users a path around missing scrollMargin. ## Changelog: [GENERAL] [FIXED] - IntersectionObserver rootMargin now affects intersection against a clipping root Pull Request resolved: https://github.com/react/react-native/pull/58177 Test Plan: - existing "clipping root with rootMargin" test: 10px margin on a 100x100 overflow:hidden root should intersect 60px of a child sitting at y=50, not 50px - new ScrollView + `rootMargin: 150px` + `threshold: [0.01]` — target fully below the fold reports intersecting - new viewport-root control — same layout, still not intersecting (intermediate ScrollView still clips) Reviewed By: rubennorte Differential Revision: D117876568 Pulled By: christophpurrer fbshipit-source-id: 381991048593959748c44e76c17a06fd8b5c0fb3 |
||
|
Jakub Piasecki
|
a35313a184 |
Add ReactNativeVersion.h to react/utils (#58403)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58403 Changelog: [General][Added] - Added `ReactNativeVersion.h` to public `react/utils` module. Existing one inside private `cxxreact/bridge` module is now deprecated and still accessible when not opted in to `RN_STRICT_API`. Reviewed By: javache Differential Revision: D119163947 fbshipit-source-id: 692649c1afce8763f7e1d1f9d642d4a652f6d075 |
||
|
Peter Abbondanzo
|
03ed284a2c |
Add experimental longest-line sizing for wrapped Text (#58351)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58351 Add `experimental_textWidthMode` as a Text style for sizing wrapped text to its widest rendered line on Android and iOS. The `auto` value preserves the existing constrained measurement behavior, while `longest-line` removes unused horizontal space without changing the line count. Changelog: [General][Added] - Add experimental longest-line width sizing for wrapped `Text` Reviewed By: javache Differential Revision: D118718410 fbshipit-source-id: aa132bf509a286ab569cc0720867eeedab215bd1 |
||
|
dagur
|
ca7acf62c5 |
Layout breaks when the first element has a different minWidth than the rest
Summary: fixes https://github.com/react/yoga/issues/2006 `distributeFreeSpaceFirstPass` decrements `totalFlexGrowFactors` / `totalFlexShrinkScaledFactors` as it freezes items, but only reduces `remainingFreeSpace` after the loop. Items after the first frozen one get an inflated fair share and freeze spuriously. When the first item clamps, the whole line freezes and `remainingFreeSpace` drains to 0, so the second pass has nothing to distribute and everything falls back to its flex basis — a 540px row of three `flexGrow: 1` items with `maxWidth: 180` and minWidths 60/30/30 lays out as 60/30/30 instead of 180/180/180. Snapshot both totals before the loop and divide by the snapshot. The first pass is then iteration 1 of CSS Flexbox §9.7, and independent of child order. Layout only changes for configs that opt in: this sits behind `Errata::FlexFirstPassUsesRunningTotals`, which new configs set by default, so existing geometry is untouched until a config clears the bit. Same shape as `MinSizeUndefinedInsteadOfAuto`, which gates the CSS §4.5 auto-min floor. Changelog: [General][Fixed] - Fix a flex line collapsing to its minimum sizes when the first item clamps to its min or max main size, behind the new `FlexFirstPassUsesRunningTotals` errata (set by default) X-link: https://github.com/react/yoga/pull/2021 Reviewed By: javache Differential Revision: D119142409 Pulled By: pasqualeanatriello fbshipit-source-id: 9e5ad19e2ca818a5cccdcac68cfb8905a6f40894 |
||
|
Jakub Piasecki
|
5464221ab8 |
Cover cxxreact:bridge with Stable API guards (#58404)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58404 Classifies `cxxreact:bridge` as a private target under the three-tier C++ stable API visibility model. Consumers that opt into `RN_STRICT_API` now get an error if they include its headers directly; without that flag the guards are inert, so no existing build changes behaviour. `ReactNativeVersion.h` is a generated file, so its guard also goes into `scripts/releases/templates/ReactNativeVersion.h-template.js`; without that the next version stamp would rewrite the header and silently drop the guard from every release and nightly artifact. Changelog: [Internal] Reviewed By: javache Differential Revision: D119163927 fbshipit-source-id: 9d436770753079448173e3bcb8d4d5e9c13ae8e3 |
||
|
ngocdevv
|
022458fbb6 |
Distinguish transparent colors from undefined props (#58093)
Summary: Fixes https://github.com/react/react-native/issues/58085. Android represents both an undefined color and explicit transparent black as ARGB `0`. `SharedColor` previously compared only that raw value, so Props 2.0 considered an explicitly supplied transparent color equal to an absent prop and omitted it from the mount diff. This change tracks color presence separately from the platform value and includes it in equality, boolean conversion, and hashing. Platform parsers and the few call sites that intentionally produce an undefined color now preserve that state explicitly. ## Changelog: [ANDROID] [FIXED] - Preserve explicitly transparent colors during Props 2.0 reconciliation. Pull Request resolved: https://github.com/react/react-native/pull/58093 Test Plan: - Added `ColorTest.testTransparentColorIsDistinctFromUndefined`. - Compiled and ran a standalone regression harness against Android `Color.cpp`; it failed before this change and passes afterward. - Built `ReactAndroid` CMake Debug for `arm64-v8a` successfully. - Validated all 9 ReactCommon, ReactAndroid, and ReactApple C++ API snapshots against Doxygen 1.16.1. - Ran targeted clang-format and `git diff --check`. Reviewed By: javache Differential Revision: D117292692 Pulled By: Abbondanzo fbshipit-source-id: 08120b7eb805dbed06cd76b6bd14fe29a556d88e |
||
|
Pieter De Baets
|
e8f50cdf28 |
Tidy the bridgeless runtime executor plumbing (#58325)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58325 Follow-ups to the buffered CallInvoker, kept out of that diff so the behavioural change stays reviewable on its own. No behaviour change intended here. - `BufferedRuntimeExecutor` gains `asRuntimeExecutor()` and `asWeakRuntimeExecutor()`. RuntimeExecutor is a std::function, so callers cannot hand it the object and instead each hand-rolled the same capturing lambda; the ownership question now has one answer per lifetime policy. The weak form is what `getBufferedRuntimeExecutor` needs, which must not keep the instance alive. - `BufferedWork` becomes private. It was only ever an implementation detail, and being public put its fields in the C++ API snapshot. - The buffer holds a `std::vector` sorted once at flush rather than a `std::priority_queue`. Indices are handed out before the lock, so arrival order can differ from submission order and something has to restore it — but a heap is the expensive way. `std::priority_queue::top()` returns a const reference, so every flushed item copied its `std::function` on the way out; that copy is now a move, and nothing sifts on the way in. - `runtimeExecutorThatGoesThroughRuntimeScheduler` was a second copy of `getUnbufferedRuntimeExecutor()`; it now calls it. - `RuntimeSchedulerCallInvoker` is marked deprecated. Its remaining users are migrated: `ReactCxxPlatform` holds a `ReactInstance` and can use `createJSCallInvoker()`, and four other files only `#include`d it without ever constructing one. The single remaining use is the flag-off branch, which is suppressed locally and goes when the flag is cleaned up. - Drops a null check on `bufferedRuntimeExecutor_` in `callFunctionOnModule` that the constructor makes unreachable. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D118616045 fbshipit-source-id: 884179f2c8a1f61185d44216a505fc75e5d103eb |
||
|
Pieter De Baets
|
3ca6ea3eca |
Buffer async CallInvoker work with module calls (#58313)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58313 In bridgeless, native reaches JS by two routes that end in the same `RuntimeScheduler` queue but get there differently. `callFunctionOnModule` goes through the instance's `BufferedRuntimeExecutor`; the `CallInvoker` goes straight to `scheduleTask`. The CallInvoker therefore skips the buffer entirely and can reach the runtime while a module call issued earlier is still parked, unflushed, because the bundle is mid-evaluation. Native code that issues both cannot rely on the order it issued them in, and `Task` is a min-heap on `now() + timeout(priority)` with no insertion tiebreak, so equal priorities do not settle it either. Gives the two channels the same buffering. `BufferedRuntimeExecutor` gains a priority-carrying `execute`, so work routed through it keeps the scheduler priority it was submitted with instead of collapsing to the executor default, and buffered work from both overloads stays in one submission-ordered stream. `BufferedCallInvoker` sits on that executor and becomes the bridgeless `jsCallInvoker` on Android, iOS and macOS. `invokeSync` deliberately keeps going straight to the scheduler: a synchronous call cannot wait for a flush that only happens once the bundle has run. Behind `enableBufferedCallInvoker`, default true. `ReactInstance` picks between the buffered invoker and the existing `RuntimeSchedulerCallInvoker` in one place, so the platform call sites are identical either way and the change is revertible at runtime — it moves when native-issued async work first reaches JS during startup, which is the intended contract but affects every native module. One lifetime hazard this surfaces, worth knowing about beyond this diff: `BufferedRuntimeExecutor` reaches the scheduler through a raw pointer captured at construction, which is safe only while the owning instance is alive. A CallInvoker is routinely held across instance teardown, so `BufferedCallInvoker` guards every async dispatch on a weak reference to the scheduler and drops the work when it has expired — the same contract `RuntimeSchedulerCallInvoker` has. Without that guard this reliably segfaults on a reload. Changelog: [General][Changed] - Async `CallInvoker` work is now buffered alongside callable module calls, so it no longer runs before the JS bundle has finished evaluating Reviewed By: rubennorte Differential Revision: D118456662 fbshipit-source-id: 7c6ccb595d69595721092eeb84b4724809140496 |
||
|
Rubén Norte
|
1a7c318ffc |
Make deprecated native module specs Flow strict-local (#57724)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57724 Upgrade the legacy native module spec files from `flow` to `flow strict-local` to enforce stricter local type checking. Loose `Object` types were replaced with the codegen-equivalent `UnsafeObject`, and `Array<any>` parameters with `Array<unknown>`, both of which are codegen-identical so the generated native interfaces are unchanged. Changelog: [Internal] Reviewed By: javache Differential Revision: D113763785 fbshipit-source-id: 085a0a570246eeda27b3c22a56a2696b04821fd5 |
||
|
nishan (o^▽^o)
|
63d118af72 |
Fix per-node memory regression caused by Grid styles (#58311)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58311 # Why Currently, grid style properties are stored in yoga style (`gridTemplateRows_`, `gridAutoColumns_` etc). These properties increase the size of style object from `152` bytes to `280` bytes (84% increase). The cost is added even when a node is not a grid container or a grid item. # How Move grid style properties behind a pointer that is lazily allocated, on the first grid property set. A node that doesn't use grid only adds the cost of this pointer (8 bytes). So style now costs 160 bytes (5% increase). The public API remains unchanged. # Tests A test is added to catch the style size regression and `tests/GridStyleTest.cpp` includes additional cases to assert unset style, copy and move behaviour. Changelog: [Internal] X-link: https://github.com/react/yoga/pull/2018 Reviewed By: rubennorte Differential Revision: D118628661 Pulled By: javache fbshipit-source-id: 185370e93bcf5b277b48c436ba3d06dada5a66fe |
||
|
Pieter De Baets
|
bf0e377c0a |
Reduce JNI allocations when importing native maps (#58274)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58274 `ReadableNativeMap` materialization copied keys and created temporary JNI references for every imported type. Cache pointers to the stable native values and reuse global `ReadableType` references so importing maps and arrays does less allocation and lookup work. Writable maps can continue mutating after materialization because `folly::dynamic` stores object entries in reference-stable `F14NodeMap` nodes. Changelog: [Internal] Reviewed By: christophpurrer, rubennorte Differential Revision: D118277119 fbshipit-source-id: 955a60ef7f1eb4cb7549d64caa1238416e90b223 |
||
|
Peter Abbondanzo
|
8cf8e094f3 |
Fold optional TextAttributes fields into a presence mask when hashing (#57984)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57984 `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Reviewed By: javache Differential Revision: D115621401 fbshipit-source-id: f164c24f6b085ef8d32cafbeb2b61636eec3d0cf |
||
|
Nicola Corti
|
49bcae1763 |
Wait for Maestro template app startup (#58289)
Summary: Replace the immediate template-app startup assertion with `extendedWaitUntil` and a 60-second timeout. The Android ARM64 debug app occasionally becomes accessibility-visible just after the existing assertion times out under native translation. This failed all three attempts in [run 33609361836](https://github.com/react/react-native/actions/runs/33609361836/job/100196578829) and [run 33540863593](https://github.com/react/react-native/actions/runs/33540863593/job/99984608541). In the captured failure artifact, both the screenshot and XML hierarchy contain `Welcome to React Native`, indicating a startup/accessibility timing race rather than an app failure. Release runs remain fast because `extendedWaitUntil` returns as soon as the element is visible. ## Changelog: [INTERNAL] [FIXED] - Wait for the template app to become visible in Maestro E2E tests. Pull Request resolved: https://github.com/react/react-native/pull/58289 Test Plan: - `MAESTRO_CLI_NO_ANALYTICS=1 maestro check-syntax scripts/e2e/.maestro/start.yml` — passed (`OK`) - `./node_modules/.bin/prettier --check scripts/e2e/.maestro/start.yml` — passed - `git diff --check` — passed - Inspected the failing Android debug artifact: the expected text is present in both the final screenshot and accessibility hierarchy. Reviewed By: christophpurrer Differential Revision: D118457123 Pulled By: cortinico fbshipit-source-id: d2f5232482933e1decb13a9b2dac7911b205db4f |
||
|
Alex Hunt
|
cec189bd34 |
Support 'react-native/react-private-interface' in build-types (#58075)
Summary: **Motivation** Minimum infra fix to enable subsequently landing https://github.com/react/react-native/pull/57940. Referencing `ReactNativeFeatureFlags` on the existing `react-native/react-private-interface` boundary is blocked by a gap in the `build-types` pipeline and a type translation error, addressed here. **Changes** - `simpleResolve.js`: Explicitly support `react-native/react-private-interface` as a special case, fixing resolution. - `ReactNativeFeatureFlagsBase.js`: Tweak the `OverridesFor` type here to fix TypeScript translation compatibility, where the unconstrained `T` type param is now narrowed. **Impact** No change to the API snapshot (the `ReactNativeFeatureFlags` import is ultimately tree-shaken!), and no effect on generated types until https://github.com/react/react-native/issues/57940 lands. Changelog: [Internal] Pull Request resolved: https://github.com/react/react-native/pull/58075 Test Plan: - `yarn build-types` - Applied https://github.com/react/react-native/issues/57940 patch on top; both stayed green, no `types_generated/**/featureflags/**` Reviewed By: GijsWeterings Differential Revision: D117513265 Pulled By: cortinico fbshipit-source-id: 348e8de3d622081fe0062ef939ce14c21af587a8 |
||
|
Nicola Corti
|
25ebffd1f7 |
Run Android E2E with ARM64 APKs (#58092)
Summary: Build only the `arm64-v8a` Android ABI for dry-run CI artifacts and run the ARM64 RNTester and template-app APKs on an API 35 `google_apis` x86_64 emulator using the system images built-in NDK translation support. This also makes the Maestro emulator API level and target configurable, logs the device ABI/native-bridge configuration, and updates local RNTester artifact selection to use the ARM64 split. Release and nightly publication builds continue to build all supported Android ABIs. ## Changelog: [INTERNAL] [CHANGED] - Run Android E2E tests with ARM64 APKs through NDK translation. Pull Request resolved: https://github.com/react/react-native/pull/58092 Test Plan: - `node --check .github/workflow-scripts/maestro-android.js` — passed. - `node --check scripts/release-testing/test-release-local.js` — passed. - Parsed all modified YAML files with the `yaml` Node package — passed. - `./node_modules/.bin/prettier --check <modified files>` — passed. - `git diff --check HEAD~3..HEAD` — passed. - `actionlint` reported only existing repository metadata warnings for the custom `4-core-ubuntu` label and the missing description in the local `yarn-install` action. - `yarn test .github/workflow-scripts/__tests__/maestro-android-test.js --runInBand` could not start because the local dependency tree is missing `flow-parser`; restoring dependencies was blocked by HTTP 503 responses from the npm registry. - The draft CI run should validate ARM64 APK installation, Hermes startup, and the complete Maestro suites through `libndk_translation.so`. Reviewed By: Abbondanzo Differential Revision: D117195078 Pulled By: cortinico fbshipit-source-id: a78f2127f04e80c3b5ca4c95d2024dffb92ff122 |
||
|
Tomasz Żelawski
|
06eb1fefab |
fix: prevent programmatic scrolls from cancelling active touches on iOS (#57546)
Summary:
On iOS, a programmatic non-animated scroll — `scrollTo` / `scrollToOffset({ animated: false })`, or any library driving the offset frame-by-frame — cancels every active touch in enclosing scroll views. Two mechanisms combine into this:
1. `scrollToOffset:animated:` calls `_forceDispatchNextScrollEvent` and, for non-animated scrolls, `_handleFinishedScrolling` — so every call emits `onScroll` (twice) plus `onMomentumScrollEnd`, bypassing `scrollEventThrottle` entirely. A per-frame driver produces a continuous stream of unthrottled `topScroll` events (~60/s measured with `scrollEventThrottle={2000}`).
2. In the responder system, any `topScroll` event without `responderIgnoreScroll: true` starts a responder negotiation, and `ScrollView`'s `onScrollShouldSetResponder` answers `true` whenever a finger is down inside it. Each event therefore steals the responder from a pressed `Pressable`/`Touchable` and the press is cancelled — `onPressIn` fires, `onPress` never does.
On Android scroll events carry `responderIgnoreScroll: true`.
I added `responderIgnoreScroll` to the C++ `ScrollEvent` payload and set it to `!_isUserTriggeredScrolling` in `_scrollViewMetrics`. Programmatic scrolls no longer transfer the responder, while user-initiated scrolls (drag, deceleration, scroll-to-top) keep today's behavior.
## Changelog:
[IOS] [FIXED] - Programmatic (non-user-initiated) scrolls no longer cancel active touches in enclosing scroll views
Pull Request resolved: https://github.com/react/react-native/pull/57546
Test Plan:
Reproducible code — an endless marquee `FlatList` nested in a `ScrollView`, driven by `requestAnimationFrame` + `scrollToOffset({ animated: false })`, with a sibling `TouchableOpacity` and a counter proving the touches reach JS:
<details><summary>App.tsx</summary>
```tsx
import { useEffect, useMemo, useRef, useState } from 'react';
import {
FlatList,
ScrollView,
Text,
TouchableOpacity,
View,
} from 'react-native';
const dpPerSecond = 20;
const size = 80;
const gap = 8;
const data = Array.from({ length: 6 }).map((_, i) => ({
id: `id-${i}`,
n: i + 1,
}));
const renderItem = ({ item }: { item: (typeof data)[0] }) => (
<View
style={{
width: size,
height: size,
backgroundColor: 'red',
justifyContent: 'center',
alignItems: 'center',
}}>
<Text>#{item.n}</Text>
</View>
);
const Carousel = ({ paused }: { paused: boolean }) => {
const ref = useRef<FlatList>(null);
const [width, setWidth] = useState(0);
const offset = useRef(0);
useEffect(() => {
if (paused) return;
const x = (size + gap) * data.length;
let last = Date.now();
let raf: number;
const loop = () => {
const now = Date.now();
offset.current =
(offset.current + (dpPerSecond * (now - last)) / 1000) % x;
last = now;
ref.current?.scrollToOffset({ offset: offset.current, animated: false });
raf = requestAnimationFrame(loop);
};
raf = requestAnimationFrame(loop);
return () => cancelAnimationFrame(raf);
}, [paused]);
const neededToFill = Math.ceil(width / (size + gap));
const extendedData = useMemo(
() => [
...data,
...data.slice(0, neededToFill).map((d) => ({ ...d, id: d.id + '-dup' })),
],
[neededToFill]
);
return (
<FlatList
scrollEnabled={false}
scrollEventThrottle={2000}
showsHorizontalScrollIndicator={false}
windowSize={3}
onLayout={(e) => setWidth(e.nativeEvent.layout.width)}
contentContainerStyle={{ gap }}
ref={ref}
horizontal
data={extendedData}
keyExtractor={(item) => item.id}
renderItem={renderItem}
/>
);
};
export default () => {
const [paused, setPaused] = useState(true);
const [touches, setTouches] = useState(0);
return (
<View style={{ flex: 1 }} onTouchStart={() => setTouches((t) => t + 1)}>
<ScrollView contentContainerStyle={{ paddingVertical: 64, gap: 32 }}>
<Carousel paused={paused} />
<TouchableOpacity onPress={() => setPaused((p) => !p)}>
<Text style={{ fontSize: 20 }}>
Try tapping me {paused ? '▶️' : '⏸️'}
</Text>
</TouchableOpacity>
<Text style={{ fontSize: 16 }}>touches seen by JS: {touches}</Text>
</ScrollView>
</View>
);
};
```
</details>
Before this change, only the first tap works (it starts the marquee); every following tap increments the touch counter but never toggles the button — the press is cancelled by the responder transfer. After this change, every tap toggles the marquee.
Recordings of the repro above (every touch is marked with a blue ring and counted on screen):
Before:
https://github.com/user-attachments/assets/bc5a8764-efe3-40ba-a9cb-a5023d140369
After:
https://github.com/user-attachments/assets/b2c1eea1-dcab-422c-9a4a-90fd69061e9d
Reviewed By: cipolleschi
Differential Revision: D116453881
Pulled By: j-piasecki
fbshipit-source-id: eb72fb0f1a4ac9fdde15f621201009d855d63890
|
||
|
Peter Abbondanzo
|
22e0eaf4e7 |
Pin clang-format 21.1.2 for OSS formatting (#58001)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58001 The existing npm `clang-format` wrapper bundles LLVM 15, causing `yarn clang-format` to disagree with the formatting of checked-in headers. Replace it with checksum-pinned DotSlash artifacts for `clang-format` 21.1.2 across Linux, macOS, and Windows. Add a chunked formatting runner and remove the obsolete npm dependency. The runner skips generated sources and ignores dirsynchronized or vendored subtrees that maintain their own formatting. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D116500359 fbshipit-source-id: 2141bee22ed2b712aae1637a1f052db23535bbb0 |
||
|
Kamil Paradowski
|
b03652af78 |
ResizeObserver Web API implementation (#57723)
Summary: > Stack 3/3 — parent: `feat/LayoutEventEmitter`. Review the two parents first. Implements the [`ResizeObserver`](https://drafts.csswg.org/resize-observer/) Web API for the New Architecture, behind the `enableResizeObserverByDefault` flag (off by default). The motivation is Web compatibility, and it's more capable than `onLayout`: callers pick which box to observe (`content-box`, `border-box`, `device-pixel-content-box`) and the sizes for those boxes are delivered in the notification. The design is as follows: JS `ResizeObserver`/`ResizeObserverEntry`/`ResizeObserverSize` on top of a manager singleton and the `NativeResizeObserver` TurboModule, using the same notify + `takeRecords` pull model. In C++, `ResizeObserverManager` collects observed targets whose layout changed at commit time (via `shadowTreeDidCommit`) and then computes and delivers observations in the event loop's "update the rendering" step (`RuntimeSchedulerResizeObserverDelegate::runResizeObservations`), as the spec requires. Requires bridgeless + the modern event loop; the legacy scheduler gets a no-op delegate. `runResizeObservations` implements the spec's depth-increasing gather/broadcast loop: each round gathers observations deeper than the shallowest target delivered in the previous round, so a callback that synchronously resizes or observes a shallower node is still delivered within the same tick. Known deviations from the spec (each pinned by a test): - Resizes triggered from a callback via a React state update are delivered on the next tick rather than within the current loop, because RN has no synchronous re-layout. Callbacks that resize or `observe()` synchronously do run further rounds in the same tick, and report `ResizeObserver loop completed with undelivered notifications` when an observation is left undelivered. - The loop carries a hard cap of 100 iterations on top of the spec's depth rule. Reaching the cap reports the loop error and stops, so a pathological callback degrades to a log line instead of a frozen app. - Re-observing a target with the same box is a no-op and does not re-deliver. This matches browsers, not the literal `observe()` algorithm. - Callback order across observers follows first-`observe()` order rather than construction order. ## Changelog: [INTERNAL] [ADDED] - Add the `ResizeObserver` API, behind the `enableResizeObserverByDefault` feature flag Pull Request resolved: https://github.com/react/react-native/pull/57723 Test Plan: - Fantom `ResizeObserver-itest.js` covers many test scenarios. - rn-tester has `ResizeObserver` examples (box sizes, text, visibility); verified initial, resize, and animation-driven delivery there. - `RuntimeSchedulerTest.cpp` new test scenarios for update rendering loop. Reviewed By: christophpurrer Differential Revision: D114045415 Pulled By: javache fbshipit-source-id: a561d5321ba45adb35f46ffb0cf26ad78eb3bae6 |
||
|
Alex Hunt
|
526016d950 |
Support committing debugger-frontend sync under Git checkouts (#57997)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57997 `scripts/debugger-frontend/sync-and-build` with `--create-diff` only worked under fbsource, making commit creation (in particular the generated changes table) inconvenient outside of Meta. This diff lifts commit creation functionality to both Git and Mercurial (`sl`), and makes writing a local commit the default behaviour. **Changes** - Synced `debugger-frontend` artifacts are now always committed, regardless of version control backend. Internal Mercurial behaviour is forked to a `fbsource-backend.fb.js` script. - `--create-diff` is narrowed to draft Phabricator diff submission (fbsource only). - The script now aborts if there are any working copy changes. Changelog: [Internal] Reviewed By: vzaidman Differential Revision: D116031207 fbshipit-source-id: 8ef3ed465b91d8f71ffa44fd1cba16f28860e3a9 |
||
|
Kamil Paradowski
|
11f9a7f449 |
RCTArrayBuffer zero-copy class for ObjC TM (#57879)
Summary: iOS TurboModules mapped a JS `ArrayBuffer` to `NSData` on arguments and `NSMutableData` on returns, so every crossing copied — and `NSMutableData` cannot alias foreign memory, so there was no way to express "these bytes live somewhere else". This adds `RCTArrayBuffer` (`packages/react-native/React/Base/`) as the ObjC representation of an `ArrayBuffer`. It carries an `isOwningBytes` flag: an owning buffer can be stored and read from any thread, a non-owning one aliases bytes valid only for the synchronous call that produced it. Codegen now emits `RCTArrayBuffer *` for `ArrayBufferTypeAnnotation` params (was `NSData *`) and returns (was `NSMutableData *`). ## Changelog: [IOS] [BREAKING] - Add `RCTArrayBuffer`, the ObjC representation of a JS `ArrayBuffer` for TurboModules, with an explicit byte-ownership contract Pull Request resolved: https://github.com/react/react-native/pull/57879 Test Plan: - `RCTTurboModuleArrayBufferTests` — 9 tests over the sync in-place path, `isOwningBytes` on a sync argument, returning one's own argument, the void/Promise copy paths, nesting, and a zero-length round trip. - `RCTTurboModuleTests.mm` adds `testNativeBackedArrayBufferIsAliasedAndKeepsBackingStoreAlive`. - `RCTSampleTurboModule` doubles its argument in place and returns the same buffer, covering the path end to end. - Codegen and C++ API snapshots regenerated. Reviewed By: cipolleschi Differential Revision: D115629409 Pulled By: christophpurrer fbshipit-source-id: a7ee6eadf99fd5f9f82fd2a95b0d3ff1369f4ddf |
||
|
Christoph Purrer
|
d84c13d511 |
Enforce the ArrayBuffer borrow contract for Java TurboModules (#57982)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57982 Changelog: [ANDROID][FIXED] Enforce the ArrayBuffer borrow contract for Java TurboModules Follow-up hardening for the Android `ArrayBuffer` TurboModule type. Three problems: **1. Borrowed JS-heap bytes outlived the call that lent them.** For a synchronous method, `convertJSIArgsToJNIArgs` hands the module a `ByteBuffer` aliasing the JS `ArrayBuffer`'s bytes without copying. Nothing stopped a module from stashing that `ArrayBuffer` in a field and reading it later, after the JS heap may have moved, freed, or reused the memory — a use-after-free that reads as intermittent data corruption rather than a crash. The borrow is now explicitly scoped to the call frame. `JNIArgs` records every borrowed `ArrayBuffer` and revokes it in its destructor — including when the call throws — via the new `JArrayBuffer::invalidate`, which drops the C++ side's reference to the bytes. `ArrayBuffer.bytes` and `ArrayBuffer.size` then throw, with a message pointing at `ArrayBuffer.arrayBufferWithCopiedBytes`, and `JArrayBuffer::toJSBuffer` throws rather than aliasing revoked memory. Modules that need the bytes past the call copy them; modules that don't keep the zero-copy fast path. Revocation lives entirely on the C++ side: the peer is the single source of truth, and Kotlin asks it through `isBytesValid`. The destructor runs while the stack unwinds, possibly with a Java exception pending, so it resolves each peer pointer at borrow time — the `global_ref` alongside it keeps the Java object, and therefore the peer, alive — and calls only the `noexcept` `JArrayBuffer::invalidate`. No JNI calls are made from the destructor, which is what lets it stay `noexcept` honestly. **2. Argument conversion aborted under runtimes that refuse `tryGetMutableBuffer`.** `jsi::Runtime::tryGetMutableBuffer` and `detached` are not universally implemented: tracing and replay runtimes throw from `tryGetMutableBuffer`, and `detached` throws a `JSINativeException` if the JS-side property isn't a bool. `ArrayBuffer` argument conversion is not wrapped in a try/catch, so either throw propagated out of a JNI frame. Both calls now go through exception-tolerant helpers in `react/bridging/ArrayBuffer.h`; a runtime that refuses to answer is treated as "no native buffer available", which selects the copy path. Routing `AsyncArrayBuffer::acquire` and `::borrow` through the same helper fixes the identical latent bug on the shared C++/ObjC path. **3. A wrong return type from a module crashed instead of raising a JS error.** The `ArrayBufferKind` return path cast the returned `jobject` to `JArrayBuffer` unconditionally. A module returning any other object type produced undefined behavior. The cast is now guarded by an `isInstanceOf` check that throws a `jsi::JSError` naming the offending module and method. Also in this change: - `JByteBufferMutableBuffer::data()` reports null for a zero-capacity direct buffer instead of calling `getDirectBytes()`, which throws for one. That made `createArrayBuffer` throw for an empty `ArrayBuffer`. - Dropped two dead zero-size branches in `JArrayBuffer`: `JByteBuffer::wrapBytes` already routes `size == 0` to an empty buffer. - `JArrayBuffer.cpp` reuses the shared `detail::OwnedBytesBuffer` from `react/bridging/ArrayBuffer.h` instead of a second local copy. - `ArrayBuffer.kt` KDoc corrected: the returned JS `ArrayBuffer` is a new object over the same bytes rather than the identical one, `size` is the capacity and not a view's remaining bytes, and `arrayBufferWithOwnedBytes` documents the caller's lifetime obligation. - `ArrayBuffer.kt` moves from the `bridge` target to `native-types`, alongside the other JNI-backed bridge types. Changelog: [Android][Breaking] - TurboModule methods taking or returning an `ArrayBuffer` now use `com.facebook.react.bridge.ArrayBuffer` instead of `java.nio.ByteBuffer`, and an `ArrayBuffer` argument must not be retained past the method that receives it unless its bytes are copied with `ArrayBuffer.arrayBufferWithCopiedBytes()`. Reviewed By: javache Differential Revision: D115794808 fbshipit-source-id: 26f5d863469cc14a3f1bffc2cbc3302f3e983ecb |
||
|
Kamil Paradowski
|
5bb9639594 |
JArrayBuffer zero-copy class for Java TM (#57897)
Summary: Android TurboModules mapped a JS `ArrayBuffer` to `java.nio.ByteBuffer`, copying every argument into a direct buffer — and `ByteBuffer` carries no ownership contract, so there was no way to express aliased or borrowed bytes for synchronous in-place access. This adds `ArrayBuffer` (`packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ArrayBuffer.kt`) as the Java representation of an `ArrayBuffer`. It carries an `isOwningBytes` flag: an owning buffer can be stored and returned to JS, a non-owning one aliases bytes valid only for the synchronous call that produced it. Codegen now emits `ArrayBuffer` for `ArrayBufferTypeAnnotation` params (was `ByteBuffer`) and returns (was `ByteBuffer`). ## Changelog: [ANDROID] [ADDED] - Add `ArrayBuffer`, the Java representation of a JS `ArrayBuffer` for TurboModules, with an explicit byte-ownership contract Pull Request resolved: https://github.com/react/react-native/pull/57897 Test Plan: - Codegen Java spec and JNI C++ snapshot tests updated for `ArrayBuffer` param/return signatures. - `SampleTurboModule` doubles its sync argument in place and returns the same buffer, covering the zero-copy path end to end; `createNativeBuffer` allocates via `ArrayBuffer`. - C++ API snapshots regenerated. Reviewed By: javache Differential Revision: D115755247 Pulled By: christophpurrer fbshipit-source-id: de067789ad145b7202da721a02f838358c82f4d8 |
||
|
Gijs Weterings
|
c467843ed0 |
Fix DOMHighResTimeStamp round-trip truncation in timing primitives (#57975)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57975 `HighResDuration::fromDOMHighResTimeStamp` and `HighResTimeStamp::fromDOMHighResTimeStamp` converted milliseconds back to nanoseconds with `static_cast<int64_t>(units * 1e6)`, which truncates toward zero. `toDOMHighResTimeStamp()` divides the nanosecond count by 1e6 (one rounding) and multiplying back by 1e6 rounds again, so the product frequently lands a hair below the original integer (e.g. `537648854729249.97`). Truncation then chops off a whole nanosecond, so `fromDOMHighResTimeStamp(toDOMHighResTimeStamp(x)) != x` for roughly 2% of random `now()` values. That is the source of an intermittent `BridgingTest/highResTimeStampTest` failure, which reported: ``` Expected equality of these values: timestamp Which is: 8-byte object <22-02 00-21 FD-E8 01-00> bridging::fromJs<HighResTimeStamp>( rt, bridging::toJs(rt, timestamp), invoker) Which is: 8-byte object <21-02 00-21 FD-E8 01-00> ``` Round to the nearest nanosecond instead of truncating. Nanosecond values below 2^53 are exactly representable in a double, so the round trip is now exact for every value below 2.25e15 ns (26 days of monotonic clock); beyond that the double's ULP exceeds 0.5 ns and residual error is at most 2 ns, which is a floor of the DOM representation itself. Rounding is also the correct semantic independently of the round trip — truncation gives a systematic downward bias and is asymmetric across zero, which affects the other callers (`RCTHighResTimeStampFromSeconds` for touch timestamps, `RuntimeTargetConsole` `console.timeStamp`, and `PerformanceTracer`). The rounding is `std::llround`, which costs both overloads their `constexpr`: `<cmath>` rounding functions do not become usable in a constant expression until C++23 (P0533), and this header is compiled as C++20 everywhere (`-std=c++20` in `rn_defs.bzl`, `react-native-flags.cmake`, the podspecs and `Package.swift`). Both carry a `TODO` to restore `constexpr` once the C++23 rollout reaches them. Dropping it is safe here. All 14 call sites are plain runtime calls — none is in a constant-expression context, none assigns to a `constexpr` variable or feeds a `static_assert` — and both functions are defined inside the class body, so they stay implicitly `inline` and neither linkage nor ABI changes. `fromDOMHighResTimeStamp` converts a `double` arriving from JS, so constant-evaluating it was never meaningful in the first place. This does narrow the published API surface, so the C++ API snapshots are regenerated: the only change across all nine `.api` files is the `constexpr` keyword dropping off these two declarations, 18 lines in total. `HighResTimeStamp::fromDOMHighResTimeStamp` now delegates to `HighResDuration`'s so there is a single implementation. `highResTimeStampTest` previously asserted on `HighResTimeStamp::now()`, whose magnitude is host-uptime-dependent, so it only tripped the bug on about 2% of runs. It now round-trips five fixed nanosecond values (including the exact value from the failing run), which exercises the bug on every run. No test was skipped, disabled, or loosened. Changelog: [General][Fixed] - Round instead of truncate when converting a `DOMHighResTimeStamp` back to nanoseconds, so `HighResTimeStamp` and `HighResDuration` round trips are exact Reviewed By: javache Differential Revision: D116286868 fbshipit-source-id: 1adafd1b57b1185f51f08fa4dc55d96b88c176cd |
||
|
Alex Hunt
|
b74adf051f |
Add idle frame support to Performance timeline (#57952)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57952 Implement Idle frame spans in React Native DevTools, by emitting synthetic `NeedsBeginFrameChanged` + `BeginFrame` events. **Definition** An idle frame is emitted whenever the gap between two consecutive frames exceeds one vsync interval, derived from the display's refresh rate (`CADisplayLink.duration` on iOS, `Display.refreshRate` on Android, falling back to 60 Hz). ``` frame N gap > 1 vsync frame N+1 +------------+ +--------------------------------+ +------------+ | BeginFrame | | NeedsBeginFrameChanged | | BeginFrame | | DrawFrame | | BeginFrame (no DrawFrame) | | DrawFrame | +------------+ +--------------------------------+ +------------+ rendered as an "Idle frame" ``` **Implementation notes** - Drop frames that begin before the recording start — Android `FrameMetrics` can deliver frames from app startup, and the first iOS `CADisplayLink` callback reports the previous vsync. - Sort frames by begin timestamp before serializing, since async screenshot encoding can deliver them out of order and break gap detection. - iOS: skip the frame event when the screenshot is unchanged, letting the gap render as an idle frame. Changelog: [Internal] Reviewed By: hoxyq Differential Revision: D97502569 fbshipit-source-id: 7cb19e0a462ad878f0b44b66f6b5de3f76426ffe |
||
|
artus9033
|
7bfe32fd33 |
Add SceneDelegate support (#57700)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57700 Add SceneDelegate lifecycle support to React Native iOS while retaining the AppDelegate path for backwards compatibility. Update linking, window resolution, bootstrap APIs, RNTester, HelloWorld, and generated API snapshots for scene-based applications. Changelog: [iOS][Added] - Add SceneDelegate lifecycle support Test Plan: - `buck2 build --flagfile fbsource//fbobjc/mode/buck2/linux --config fbobjc.react_native_debug=development --flagfile fbsource//fbobjc/mode/iphonesimulator-arm64 --flagfile fbsource//fbobjc/mode/jest-coverage --config xplat.available_platforms=CXX,APPLE --config cxx.default_platform=iphonesimulator-arm64 --config user.sandcastle_build_mode=profile --config user.platform_flavor=iphonesimulator-arm64 //xplat/js/react-native-github/packages/rn-tester:RNTesterIOS` - `xcodebuild -quiet -workspace RNTesterPods.xcworkspace -scheme RNTester -configuration Debug -destination "generic/platform=iOS Simulator" CODE_SIGNING_ALLOWED=NO build` - `arc lint` - Evaluate RNTester with the `RNTester` SceneDelegate scheme. - Evaluate RNTester with the `RNTester (AppDelegate)` legacy scheme. - Verify HelloWorld bootstrap and deep linking. Reviewed By: cortinico, javache Differential Revision: D113758228 Pulled By: cipolleschi fbshipit-source-id: 31174c03a0936ca397fc1d061650fc0f582bdda7 |
||
|
Sam Zhou
|
727e87ed26 |
Fully replace hermes-(estree|parser|eslint|transform) packages with flow-* counterparts in react-native (#57851)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57851 The flow-* packages have support for all latest syntax and is better maintained. Changelog: [Internal] Reviewed By: huntie Differential Revision: D115064040 fbshipit-source-id: 7265eb722910a460c76d0dbde0603cf962fe4e4e |
||
|
Evan Katz
|
8415753e21 |
Add variable font settings support (#57815)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57815 Apply the existing `fontVariationSettings` text style prop when Fabric constructs fonts on iOS. Parse CSS-compatible axis settings into CoreText variation dictionaries while preserving absent, explicit-clear, and invalid value semantics for nested text. The parser supports quoted four-character OpenType tags and finite numeric values, rejects malformed settings as a complete unit, and applies normalized variations after the base font and feature settings are resolved. This shared attributed-text path covers Fabric `Text` and `TextInput`. Changelog: [iOS][Added] - Add `fontVariationSettings` support for Fabric text Reviewed By: Abbondanzo, christophpurrer Differential Revision: D114121940 fbshipit-source-id: d2b2fffd4fe723c5205e5279a466a125aa7edd38 |
||
|
Evan Katz
|
19f7d144b9 |
Add Android text font variation settings (#57804)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57804 Add a `fontVariationSettings` text style prop and carry it through Fabric text attributes into Android text rendering. Android now deserializes the prop for `<Text>`, applies it to `Paint`, and includes it in text measurement cache identity because variable axes can affect layout. Preserve the distinction between an absent setting and an explicitly empty setting so nested text can inherit or clear the parent variation axes. Apply high-level font properties before low-level variation settings so explicit axes take precedence, matching CSS font realization order. Settings syntax is intentionally forwarded unchanged through common text attributes and validated only by the Android font variation parser. This avoids narrowing the grammar Android accepts. As a result, malformed child settings are outside the supported inheritance contract: they replace an inherited value before Android validation and are not guaranteed to fall back to the parent settings. Android also accepts `normal` and the React Native empty-string convention as explicit resets. Changelog: [Android][Added] - Add `fontVariationSettings` support for `<Text>` Reviewed By: Abbondanzo, huntie Differential Revision: D113580491 fbshipit-source-id: 46cea13ef5f06b33ed9f9d0b28b5c16f84193ea6 |
||
|
Pieter De Baets
|
85a81818c6 |
Avoid copying surface props when starting or updating a surface (#57813)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57813 `SurfaceHandler` already takes a throwaway snapshot of its `Parameters` under `parametersMutex_` before handing them to the `UIManager`, but `UIManager::startSurface` and `UIManager::setSurfaceProps` took `moduleName` and `props` by const reference and then copy-captured them into the lambda posted to the `RuntimeExecutor`. That forced a second deep copy of the props tree — which for a real surface holds the initial route params and deep link data — on every surface start, prop update, and display mode change. Take both by value and move them into the lambda, and move at the `SurfaceHandler` call sites, so the snapshot is handed off instead of duplicated. The snapshot is a local that is dead after the call, so there is nothing left to observe the moved-from state. Changelog: [General][Changed] - `UIManager::startSurface` and `UIManager::setSurfaceProps` now take `moduleName` and `props` by value Reviewed By: zeyap, christophpurrer Differential Revision: D114730310 fbshipit-source-id: 7891f6ac61abda43ff0da5f4336741af6c329ed2 |
||
|
Tomasz Żelawski
|
e2a4c68449 |
refactor(iOS): persist bundle as local file on par with Android (#57751)
Summary: On Android, in development, the JS bundle is downloaded and saved as a file - this is done to avoid passing it through JNI, but iOS could use similar approach for uniform behavior. This allows libraries to be able to obtain the bundle from the file without re-downloading it from Metro. It would impact iOS boot slightly, but only in development, so it should be negligible. ## Changelog: [IOS] [CHANGED] - make iOS preserve dev bundle as a temp file on par with Android implementation Pull Request resolved: https://github.com/react/react-native/pull/57751 Test Plan: rn-tester iOS dev works with this change Reviewed By: cortinico, christophpurrer Differential Revision: D114346244 Pulled By: coado fbshipit-source-id: ebb4264fe8949cd766e71ca0363aa209a61370f7 |
||
|
Kamil Paradowski
|
5fb3ebce1a |
ArrayBuffer support to Java TurboModules
Summary: Changelog: [ANDROID] [ADDED] - Add ArrayBuffer support to Java TurboModules - Codegen: ArrayBufferTypeAnnotation → ByteBuffer / ArrayBufferKind / Ljava/nio/ByteBuffer; in GenerateModuleJavaSpec and GenerateModuleJniCpp. Guard in Utils.js rejects Promise<ArrayBuffer> on Android at codegen time — drops async return to avoid CxxCallbackImpl.kt / JCallback / PromiseImpl.kt changes. - Runtime: New JByteBufferMutableBuffer.h — jsi::MutableBuffer wrapping global_ref<JByteBuffer> direct ByteBuffer, holds Java object alive for JS ArrayBuffer lifetime; dtor uses ThreadScope to attach JNI thread (Hermes GC finalizes off-thread). Zero-copy native→JS return mirrors iOS NSMutableDataBuffer. Args JS→Java always copied into Java-owned direct ByteBuffer via allocateDirect+memcpy (mirrors iOS NSData copy), no sync/async branching — safe to retain/dispatch past GC. - No changes to CxxCallbackImpl.kt, JCallback, PromiseImpl.kt. Nested ArrayBuffers still via folly::dynamic, deferred like ObjC/C++. - Samples: NativeSampleTurboModule.js spec, SampleTurboModule.kt (getArrayBuffer/ createNativeBuffer/ processAsyncBuffer), RCTSampleTurboModule.mm. (NSData param / NSMutableData return), RNTester UI SampleTurboModuleExample.js. X-link: https://github.com/facebook/react-native/pull/57062 Reviewed By: javache Differential Revision: D107411163 Pulled By: christophpurrer fbshipit-source-id: 67cc5da796a87f6918f0382debc5c5594f76661b |
||
|
Christoph Purrer
|
3f3425e246 |
Remove unused NativeModalManager spec and all related code (#57806)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57806 Changelog: [INTERNAL] Remove the dead `NativeModalManager` TurboModule spec and all its dependents — 13 files, 192 deletions across JS spec, Modal.js usage, native registration, CXX API snapshots, blocklist entries, and test mocks. The spec was never implemented and the Modal component no longer needs the event subscription pattern since transitioning to the new renderer. Reviewed By: cortinico Differential Revision: D114652881 fbshipit-source-id: af939c002d4d58c662bacb840c805d2dd4f42eb4 |
||
|
Hanno J. Gödecke
|
f535c97904 |
fix(android): props 2.0 image tintColor=transparent broken (#57668)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57668 When enabling the props 2.0 feature flags I noticed that for an `<Image source={...} style={{ tintColor: 'transparent' }} />` the image is actually still showing, instead of becoming transparent. ### The underlying issue All color props are defined as `SharedColor`, where a `SharedColor` has a default value of `0`: https://github.com/facebook/react-native/blob/f3678f51d9873cb19602d7e36a4d8ed71562b9d0/packages/react-native/ReactCommon/react/renderer/graphics/platform/android/react/renderer/graphics/HostPlatformColor.h#L15-L18 https://github.com/facebook/react-native/blob/f3678f51d9873cb19602d7e36a4d8ed71562b9d0/packages/react-native/ReactCommon/react/renderer/graphics/Color.h#L30-L32 > [!NOTE] > This is a bit confusing to me. `0` is not really an "undefined" color, but its actually "transparent". What we are really saying this way is that all color props have a default value of "transparent". The naming makes me unsure whether this has been intentional. For other use cases, this seems to make sense. Ie. a user expects their text to have a background color of transparent/"nothing". However, we do not expect our image's tint color to have a default color of "transparent". This would hide all our images. With props 2.0 we use the `getDiffProp` function, and when we pass `tintColor: 'transparent'` it will be passed to native as `tintColor: 0`. When we then compare the passed prop's value vs the default value here, it will not include the `tintColor`: https://github.com/facebook/react-native/blob/f3678f51d9873cb19602d7e36a4d8ed71562b9d0/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp#L249-L251 `tintColor` really is an optional color prop, and should be treated as such. The best fix I found was therefor making it really an `std::optional`. Let me know if you think otherwise! ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [ANDROID] [CHANGED] - ImageProps make `tintColor` an `std::optional` to support color `transparent` (`0`) with props 2.0 X-link: https://github.com/facebook/react-native/pull/55535 Test Plan: - In the RNTester app change one of the Tint Color image examples to use tintColor of "transparent" - The image should be invisible now as all visible pixels turned transparent - Enable the props 2.0 feature flags - Run the same example, notice that the tintColor has not been applied Reviewed By: lenaic Differential Revision: D93140534 Pulled By: coado fbshipit-source-id: 0d8abf3583d87ad840b3f72603b4c29c775671ca |
||
|
Rob Hogan
|
fc365f6e72 |
Flow: Prefer imported types from @babel/types over global BabelNode* types (#57752)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/57752 ## Rationale Flow lib defs that declare global types (available anywhere, without an `import`) must be referenced in `.flowconfig` and can't be maintained incrementally. That's not too bad for very stable APIs and it's necessary for environment/runtime globals, but for 3P libraries it makes the lib defs much more difficult to maintain for little benefit (we have to import the runtime APIs anyway). Secondarily, it's a problem for generating TypeScript types, as TS doesn't declare any 3P library globally. Babel is one of few cases where a library declares Flow globals - every one has an importable equivalent. ## This diff Replaces usages of Babel global types across xplat/js with their `babel/types` equivalents Changelog: [Internal] Reviewed By: javache Differential Revision: D113574665 fbshipit-source-id: be668d968345a60e76a3515d19d8eaa002d53274 |