Fix errors with component stacks reported as warnings #46637
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D63349613 |
|
This pull request was exported from Phabricator. Differential Revision: D63349613 |
rickhanlonii
force-pushed
the
export-D63349613
branch
from
10ad9d3
to
ec14c17
Compare
rickhanlonii
added a commit
to rickhanlonii/react-native
that referenced
this pull request
Sep 26, 2024
Summary: Pull Request resolved: facebook#46637 Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Reviewed By: huntie Differential Revision: D63349613
|
@rickhanlonii has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Pull Request resolved: facebook#46637 Reviewed By: huntie Differential Revision: D63349613 Pulled By: rickhanlonii
|
This pull request was exported from Phabricator. Differential Revision: D63349613 |
rickhanlonii
force-pushed
the
export-D63349613
branch
from
ec14c17
to
3539264
Compare
|
@rickhanlonii merged this pull request in cbb3132. |
|
This pull request was successfully merged by @rickhanlonii in cbb3132 When will my fix make it into a release? | How to file a pick request? |
blakef
pushed a commit
that referenced
this pull request
Oct 7, 2024
Summary: Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Pull Request resolved: #46637 Reviewed By: huntie Differential Revision: D63349613 Pulled By: rickhanlonii fbshipit-source-id: 32e3fa4e2f2077114a6e9f4feac73673973ab50c
okwasniewski
added a commit
to callstack/react-native-visionos
that referenced
this pull request
Oct 21, 2024
* fix: pass closest public instance to React DevTools (#46664)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46664
# Changelog: [Internal]
Please see https://github.com/facebook/react/pull/31068.
Synced changes for React Native - D63453667.
Reviewed By: huntie
Differential Revision: D63421463
fbshipit-source-id: e455711206598a06f7cdce4150e79c3548843b99
* Use native module to persist RDT settings (#46663)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46663
# Changelog: [Internal]
Now we can use the native module for persisting settings in React DevTools. Basically, this diff adds read / write implementations.
Reviewed By: huntie
Differential Revision: D62967060
fbshipit-source-id: c14ef056c8d7e30a23d2b84d1ad4fc0ad8eaaf34
* Unbreak Android LayoutAnimation by making all surfaces go through Binding (#46691)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46691
In Bridgeless, ReactSurface would only call `registerSurface` when it started, which did not match the behaviour seen in startSurface(WithConstraints). Instead, make all calls to start the surface go through the FabricUIManager Binding so we can correctly configure `setMountingOverrideDelegate` which is required for layout animations.
Changelog: [Android][FIxed] LayoutAnimations work on full new architecture
Reviewed By: cortinico
Differential Revision: D63533635
fbshipit-source-id: f6d3db020bb2d7245f7b14f2407271d76837d40c
* RNGP: Read `enableWarningsAsErrors` property correctly (#46657)
Summary:
I've noticed that some users are reporting build failures due to warnings inside RNGP.
We do have `allWarningsAsErrors` set to true for everyone (also for users).
That's too aggressive, and can cause build failures which are not necessary. Let's keep it enabled only on our CI (when the `enableWarningsAsErrors` property is set).
## Changelog:
[INTERNAL] - RNGP: Read `enableWarningsAsErrors` property correctly
Pull Request resolved: https://github.com/facebook/react-native/pull/46657
Test Plan: CI
Reviewed By: NickGerleman
Differential Revision: D63459601
Pulled By: cortinico
fbshipit-source-id: 0307e8d6771518038a5abe27ca5a993cb0a9f8c0
* Switch to hermes-parser in eslint-* package tests (#46699)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46699
Switch from legacy Babel Flow parser integrations to the Meta-maintained `hermes-eslint` and `babel-plugin-syntax-hermes-parser` packages (both part of the `hermes-parser` codebase).
Required to unblock D63535216.
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63541483
fbshipit-source-id: 04ccfa04c9a2b8c0a87ef1a5c38e952971838b77
* Switch Babel parsing from legacy Flow plugin to hermes-parser (#46696)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46696
Following D62161923, we began to lose sync with modern Flow syntax when Metro's `transformer.hermesParser` option is disabled. This config option loads Babel for transformation (instead of `hermes-parser`), which requires a Babel plugin to parse (not strip) Flow syntax.
This diff migrates us away from `babel/plugin-syntax-flow` (see also https://github.com/babel/babel/issues/16264) and uses the modern [`babel-plugin-syntax-hermes-parser`](https://www.npmjs.com/package/babel-plugin-syntax-hermes-parser) instead (a component of the modern Hermes Parser stack).
Following this change, new projects that unset `transformer.hermesParser` will compile.
Resolves https://github.com/facebook/react-native/issues/46601.
Changelog:
[General][Fixed] - Fix parsing of modern Flow syntax when `transformer.hermesParser = false` is configured in Metro config
Reviewed By: cipolleschi
Differential Revision: D63535216
fbshipit-source-id: d2c6ddec030d89e2698e03b76194cf3568d04e6b
* Switch to hermes-parser in eslint-config
Summary:
Similar to D63541483, modernises our Flow syntax support for our published ESLint config to use `hermes-eslint` (`hermes-parser`).
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63541856
fbshipit-source-id: 06cc5725faf5934fda07713ec1dac54ff9c32ddf
* Substitute babel-preset-fbjs for metro-transform-plugins (#46689)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46689
Resubmission of D61014834 / https://github.com/facebook/react-native/pull/45959 (previously reverted).
> Addresses TODO comment. I noticed this lone reference to babel-preset-fbjs (last published 3y ago) while attempting to upgrade our Babel Flow syntax plugin.
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63532211
fbshipit-source-id: e94b55b7b61194b50131b36a2fe817e149152953
* Add minimal Flow types to remaining files under Libraries/ (#45785)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45785
Adds `flow` annotation, and/or adds minimal Flow types to remaining files under `packages/react-native/Libraries/`. As of this diff, 100% of this directory is now parsable by `flow-api-translator` and covered by `public-api-test`.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D60377082
fbshipit-source-id: 67e8acafc3fdd8f107f1d8d53c69ee6c27d3ed0a
* Require Flow types in all react-native source files (#45786)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45786
Following D60377082, `packages/react-native/Libraries/` is now 100% parsable by `flow-api-translator` and covered by `flow-api-test`. This diff increases test strictness to preserve this state going forward.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D60377123
fbshipit-source-id: 8c48bdd73d9d0d97a114d553eb70cc01faf18e89
* Fabric: Fixes animations strict weak ordering sorted check failed (#46582)
Summary:
Fixes https://github.com/facebook/react-native/issues/46568 . cc cipolleschi
## Changelog:
[IOS] [FIXED] - Fabric: Fixes animations strict weak ordering sorted check failed
Pull Request resolved: https://github.com/facebook/react-native/pull/46582
Test Plan:
See issue in https://github.com/facebook/react-native/issues/46568
## Repro steps
- Install Xcode 16.0
- navigate to react-native-github
- yarn install
- cd packages/rn-tester
- bundle install
- RCT_NEW_ARCH_ENABLED=1 bundle exec pod install
open RNTesterPods.xcworkspace to open Xcode
{F1885373361}
Testing with Reproducer from OSS
| Paper | Fabric (With Fix) |
|--------|-----------------|
| {F1885395747} | {F1885395870} |
Android - LayoutAnimation (Looks like it has been broken and not working way before this changes.)
https://pxl.cl/5DGVv
Reviewed By: cipolleschi
Differential Revision: D63399017
Pulled By: realsoelynn
fbshipit-source-id: aaf4ac2884ccca2da7e90a52a8ef10df6ae4fc8a
* Deploy 0.247.1 to xplat (#46710)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46710
Changelog: [Internal]
Reviewed By: SamChou19815
Differential Revision: D63560047
fbshipit-source-id: a43ac647fee12115c794aa8002bd340a4c4bcf16
* Back out "Do not hold raw RuntimeScheduler pointer in BufferedRuntimeExecutor"
Summary:
A crash we are getting in the wild suggests that destruction on weak ref count going away may be delaying RuntimeExecutor task destruction to a point where jsi::Function is invalid. Let's try backing D62748768 out and seeing if the crash goes away.
Changelog:
[General][Fixed] - Attempt to fix crash from delayed RuntimeExecutor task destruction
#bypass-github-export-checks
Reviewed By: mdvacca
Differential Revision: D63568504
fbshipit-source-id: 6152e7293902d0eb67a14c5840bea56561c35b08
* Fix interactions between removeClippedSubviews and RTL by applying HorizontalScrollContentView translation during layout metric assignment (#46685)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46685
This attempts a similar fix to D59566611, but keeps the removedClippedSubviews logic as is, instead moving the current translation done in ReactHorizontalScrollContainerView onLayout to ShadowNode layout metric assignment (so the world is consistent from very early on, and `removeClippedSubviews` never sees coordinates before translation),
I suspect doing this at the ShadowNode layer might also result in some fixes to DevTools in RTL.
This should let us roll out setAndroidLayoutDirection again.
Changelog:
[Android][Fixed] - Fix interactions between removeClippedSubviews and RTL
Reviewed By: mdvacca
Differential Revision: D63318754
fbshipit-source-id: 828e103e2ad21c7e886e39c163474b10ebd5099e
* Remove `experimental_` and cleanup mixBlendMode experiment (#46650)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46650
tsia
Changelog: [General] [Added] - Removed `experimental` prefix and fully released `mixBlendMode` prop
Reviewed By: joevilches
Differential Revision: D63407473
fbshipit-source-id: 892ababcfe19b82d3f4c73d3cbc13fb46b7425b6
* Fix mixBlendMode not updating on Image state updates (#46680)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46680
Since the parent of a view with mixBlendMode prop set is the one responsible for setting the compositing of the child, when updating mixBlendMode on a child we need to also invalidate the parent.
Changelog: [Android] [Fixed] - mixBlendMode now properly does state updates
Reviewed By: joevilches
Differential Revision: D63424394
fbshipit-source-id: 0eb15520f1087e25683853632943e64a66344481
* Silence unnecessary Gradle outputs - Part 1 (#46704)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46704
Our build log for Gradle is extremely noisy due to Hermes.
Here I'm suppressing all the build output from Hermes as we can't really do much from that side of the build.
This should make it easier for folks on GitHub Actions to immediately spot where are failures.
Changelog:
[Internal] [Changed] - Silence unnecessary Gradle outputs
Reviewed By: GijsWeterings
Differential Revision: D63541175
fbshipit-source-id: d1a60098c317ff9e8c9575b5b8b2aab639f28f2f
* delete state alignment mechanism (#46658)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46658
changelog: [internal]
We are shipping a better solution: Runtime Shadow Node Reference Syncing which is being rolled out.
Reviewed By: lenaic, rubennorte
Differential Revision: D63456344
fbshipit-source-id: e70b7dd4be7bf0670366e5200a910195b929a14d
* Fix the generation of .xcode.env.local (#46661)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46661
The previous approach was brittle and it was not working in all the scenarios.
This is the same approach used [by Expo](https://github.com/expo/expo/blob/12f24ea7fdbc8bab864d7852ae8e7275e44db4df/packages/expo-modules-autolinking/scripts/ios/xcode_env_generator.rb#L37C44-L37C75) (thanks guys! :D) and it looks like it is more stable.
This should definitely fix [#43285](https://github.com/facebook/react-native/issues/43285).
## Changelog
[Internal] - Fix the generation of .xcode.env.local
Reviewed By: cortinico
Differential Revision: D63460707
fbshipit-source-id: c6732adce3df5f8365b17ed9c500c38f773ecee5
* Make more SurfaceHandler configuration go through UIManager Binding (#46701)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46701
Reduce the interface we expose in SurfaceHandler(Binding), and concentrate the logic in the FabricUIManager Binding.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D63536328
fbshipit-source-id: 5882bdd24fd3ca8f3d36210dca80587fee091dcf
* Rename Binding to FabricUIManagerBinding (#46705)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46705
We have multiple classes in React Native Android acting as a binding layer (between C++ and Java, between C++ and JS). Make it more obvious this Binding is for (Android) FabricUIManager and flatten the (now) unused Binding interface.
Changelog: [Android][Removed] BindingImpl is no longer part of the public interface
Reviewed By: cortinico
Differential Revision: D63536329
fbshipit-source-id: 329d184c1889fbe804995211cdd339b50a7c9234
* fix(iOS): title and title color handling added for refresh control (#46655)
Summary:
Solve a part of this issue: https://github.com/facebook/react-native/issues/46631
## 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
-->
[IOS] [CHANGED] - Passed correct title and titleColor prop to updateTitle function
**What's the Issue:**
When updating the PullToRefreshViewProps in a React Native iOS app, changes to the title and titleColor were not being reflected properly in the RefreshControl. This happened because the function responsible for updating the title (_updateTitle) was not always receiving the correct or updated values for title and titleColor.
**Updated `_updateTitle` function:**
The _updateTitle method was modified to accept both title and titleColor as parameters. This ensures that the latest values are always used when updating the refresh control's attributedTitle.
If the title is empty, the attributedTitle is cleared by setting it to nil. Otherwise, both the title and titleColor (if present) are applied correctly.
Pull Request resolved: https://github.com/facebook/react-native/pull/46655
Test Plan:
**Without fix:**
https://github.com/user-attachments/assets/8a83c247-bf78-4080-bdc1-ac5a852481e8
**With Fix:**
https://github.com/user-attachments/assets/52e2495a-4419-41d1-b308-acb64600f9f7
Reviewed By: javache
Differential Revision: D63466516
Pulled By: cipolleschi
fbshipit-source-id: fef61a003b658b20a25b61b6d07ee9fe0750dae7
* add missing iOS platform colors (#46730)
Summary:
- add missing platform colors on iOS https://developer.apple.com/documentation/uikit/uicolor/standard_colors
## Changelog:
[IOS] [ADDED] - Add `systemCyan` and `systemMint` colors on iOS
<!-- 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
Pull Request resolved: https://github.com/facebook/react-native/pull/46730
Test Plan:
- Ran it in an iOS app (text is no longer black)
Reviewed By: cortinico
Differential Revision: D63635182
Pulled By: cipolleschi
fbshipit-source-id: b6fa3ff0e38630d820aacedbf88dfe9bd13c0644
* feat(iOS): refactor RCTDevLoadingView to use constraints (#46621)
Summary:
This PR refactors the RCTDevLoadingView to use constraints instead of explicitly defining frames.
Now the code looks like this:
```
#if TARGET_OS_MACCATALYST
self->_window.frame = CGRectMake(0, window.safeAreaInsets.top, windowWidth, 20);
self->_label = [[UILabel alloc] initWithFrame:CGRectMake(0, 0, windowWidth, 20)];
#else
self->_window.frame = CGRectMake(0, 0, windowWidth, window.safeAreaInsets.top + 10);
self->_label = [[UILabel alloc] initWithFrame:CGRectMake(0, window.safeAreaInsets.top - 10, windowWidth, 20)];
#endif
```
When working with visionOS I quickly found my self in the need of adding additional else there...
This PR uses constraints to layout the views correctly for every device.
| Device | Screenshot |
| -------------- | ---------- |
| iOS | <img src="https://github.com/user-attachments/assets/decc146e-41a2-4ce8-bafd-77c77a39398b" width="200"/> |
| macOS Catalyst | <img src="https://github.com/user-attachments/assets/78ef4946-a6dc-49a2-864e-a483577798f1" width="200"/> |
| iPadOS | <img src="https://github.com/user-attachments/assets/a15bd1a8-84ae-47d0-becb-e14c3a9352d4" width="200"/> |
| visionOS | <img src="https://github.com/user-attachments/assets/d74f0607-837e-4f7b-b924-fc6fce80d763" width="200"/> |
## Changelog:
[IOS] [CHANGED] - refactor RCTDevLoadingView to use constraints
Pull Request resolved: https://github.com/facebook/react-native/pull/46621
Test Plan: Test if DevLoading view is properly showing up
Reviewed By: blakef
Differential Revision: D63383002
Pulled By: cipolleschi
fbshipit-source-id: b17499e12d4a882facce4c45b1cc22eb43ea3bb1
* Fix errors with component stacks reported as warnings (#46637)
Summary:
Ok so this is a doozy.
## Overview
There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally).
However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case.
In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false.
However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning.
## What's the fix?
Change the default settings for the warning filter.
## What's the root cause?
Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests)
## How could it have been caught?
It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on.
Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/46637
Reviewed By: huntie
Differential Revision: D63349613
Pulled By: rickhanlonii
fbshipit-source-id: 32e3fa4e2f2077114a6e9f4feac73673973ab50c
* fix(dev-menu): export for react-native package (#46723)
Summary:
In an effort to achieve a similar API in the new architecture, I was trying to replace the following code from the old architecture:
```
import { NativeModules } from 'react-native'
// ...
NaiveModules.DevMenu.show()
```
After https://github.com/facebook/react-native/issues/46694 landed, you can achieve this with the following import in the new architecture:
```
// ts-ignore
import DevMenu from "react-native/Libraries/NativeModules/specs/NativeDevMenu"
// and then use it
DevMenu.show()
```
However, this change provides the interface for `DevMenu` from the `react-native` package, similar to the API for `DevSettings`, making it easier to use via the public API and better type support.
```
import { DevMenu } from 'react-native'
DevMenu.show()
```
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[GENERAL] [ADDED] - Export `DevMenu` from `react-native`
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: https://github.com/facebook/react-native/pull/46723
Test Plan:
<img width="750" alt="image" src="https://github.com/user-attachments/assets/dc5a9876-4364-473e-9fd8-c49484d6de01">
### Before
`any` type on `show` from the private import
<img width="226" alt="image" src="https://github.com/user-attachments/assets/45a085d2-9794-482d-b5ec-3b882aadb56b">
### After
typed show method from the new import
<img width="399" alt="image" src="https://github.com/user-attachments/assets/7c7587b3-499b-40f0-8262-126e4ec17dc6">
Reviewed By: cortinico
Differential Revision: D63631196
Pulled By: huntie
fbshipit-source-id: a00efa85c8ca1aa127c9ae0a059978ba5eadb376
* Use HybridClassBase for frequently constructed objects (#46706)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46706
Simplifies object construction and collection, and avoids additional JNI roundtrips to set HybridData.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D63537781
fbshipit-source-id: 55df2785840ec8fcd5a7a08d2c7dd73c5b5fca82
* Add a test for an Alias to an Alias (#46725)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46725
This covers types that deeply alias.
Changelog: [Internal]
Reviewed By: GijsWeterings
Differential Revision: D62926913
fbshipit-source-id: 2fea1900f1be2893230a949db6778c76afae0423
* Add test for Object with indexers (#46726)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46726
An object isn't allowed to have both an indexer and regular properties. Previously, the schema just wouldn't include the properties and would only include the indexer. Instead of failing silently and not generating the expected code, let's explicitly error out.
Changelog: [Internal]
Reviewed By: GijsWeterings
Differential Revision: D63615090
fbshipit-source-id: a4c881b7a809f0b467509f7f7e2131be59ee274e
* Promise annotations must specify their elementType (#46727)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46727
It makes scripts operating on the schema complicated when the elementType might be there or might not. Let's make it required, but VoidTypeAnnotation if it's unknown.
Arguably we shouldn't allow it to be unknown at all, but that's out of scope here.
Changelog: [Internal]
Reviewed By: GijsWeterings
Differential Revision: D63616703
fbshipit-source-id: 290586384b911928e55344aa522bd296f23a074c
* RN: Remove String Refs from `NativeMethods` (TS) (#46734)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46734
The TypeScript definition for `NativeMethods` defined a `refs` property for referencing component instances by string refs.
String refs are deprecated and support is being formally dropped in React 19.
This removes the obsolete type definition.
Changelog:
[General][Removed] - Removed `refs` property from `NativeMethods` TypeScript definition.
Reviewed By: NickGerleman
Differential Revision: D63640881
fbshipit-source-id: 82148ac91bd44d63da9a41b1d1dcebb7ca32a5bd
* Further Reduce Build Warnings (#46733)
Summary:
This further reduces the build warning coming from the Hermes build, which we can't do much about as we don't own that code.
## Changelog:
[Internal] [Changed] - Further Reduce Build Warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/46733
Test Plan: CI
Reviewed By: NickGerleman
Differential Revision: D63644753
Pulled By: cortinico
fbshipit-source-id: d0e65559715ac1fa2b920f4c3d3cf63b6171765d
* fix(react-native-codegen): upgrade `jscodeshift@17.0.0` to resolve outdated dependencies (#46724)
Summary:
This upgrades `jscodeshift` from `0.14.0` to `17.0.0`, to resolve unsupported babel dependencies when installing React Native. See https://github.com/expo/expo/issues/31613
## Changelog:
[GENERAL] [FIXED] - Upgrade Codegen dependency `jscodeshift@17.0.0` to resolve outdated dependencies
Pull Request resolved: https://github.com/facebook/react-native/pull/46724
Test Plan:
To test it out yourself:
- `npx create-expo@latest ./test-codegen`
- `cd ./test-codegen`
- `npm why babel/plugin-proposal-nullish-coalescing-operator@7.18.6 | grep codegen` - [npm deprecation](https://www.npmjs.com/package/babel/plugin-proposal-nullish-coalescing-operator/v/7.18.6)
- `npm why babel/plugin-proposal-class-properties@7.18.6 | grep codegen` - [npm deprecation](https://www.npmjs.com/package/babel/plugin-proposal-class-properties/v/7.18.6)
- `npm why rimraf@2.6.3 | grep codegen` - [npm deprecation](https://www.npmjs.com/package/rimraf/v/2.6.3)
- `npm why babel/plugin-proposal-optional-chaining@7.21.0 | grep codegen` - [npm deprecation](https://www.npmjs.com/package/babel/plugin-proposal-optional-chaining/v/7.21.0)
> [!IMPORTANT]
> There are other dependencies that require an update, unfortunately there are no flow typings available (yet).
> - `npm why glob@7.2.3 | grep codegen` - [npm deprecation](https://www.npmjs.com/package/glob/v/7.2.3)
Reviewed By: cortinico
Differential Revision: D63629133
Pulled By: huntie
fbshipit-source-id: 3799e49677b8e5fd64841e6c7f855bde10e15072
* Add StrictAnimatedProps to fix missing Animated types (#46659)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46659
The current `AnimatedProps` maps all types to `any` which prevents us from catching many types of issues. Introduce an alternative variant which we can adopt incrementally which enforces the underlying types.
Changelog: [General][Fixed] Improved types for AnimatedProps
Reviewed By: SamChou19815
Differential Revision: D63315760
fbshipit-source-id: 718fc2754cc99891e490a48d7792f626205c37e4
* Reorganise debugger modules in src/private/ (#46738)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46738
Combines modules previously under `fusebox` and `reactdevtools` directories into a single `src/private/debugging/` directory.
Changelog: [Internal]
Reviewed By: rubennorte
Differential Revision: D63637625
fbshipit-source-id: 58add7fa3b6553c33d59e785563b24d57b7db547
* Simplify FuseboxReactDevToolsDispatcher diagram and document inline (#46737)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46737
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63637626
fbshipit-source-id: 314ec6dc6eff8d458beba84e49d5aaca3d192087
* feat(iOS): expose RCT_NEW_ARCH_ENABLED to Swift (#46749)
Summary:
This PR exposes `RCT_NEW_ARCH_ENABLED` flag to Swift.
It allows you to use conditional compilation:
```swift
#if RCT_NEW_ARCH_ENABLED
func test() {
print("I'm running on new arch!)
}
#else
func test() {
print("I'm running on old arch!)
}
#endif
```
## Changelog:
[IOS] [ADDED] - expose RCT_NEW_ARCH_ENABLED to Swift
Pull Request resolved: https://github.com/facebook/react-native/pull/46749
Test Plan: CI Green
Reviewed By: cortinico
Differential Revision: D63689711
Pulled By: cipolleschi
fbshipit-source-id: 706013019571f597d1966ffcf13bd47ba7f29a2d
* Fix BoxShadow support for platform colors (#46753)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46753
Colors can be more than just ints, so we need to use `ColorPropConverter` to convert it correctly. For ViewManagers, this happens automatically through the ReactProp annotation, but since this is a nested field, we need to manually apply it.
Changelog: [Android][Fixed] BoxShadow now supports platformColor.
Reviewed By: fabriziocucci
Differential Revision: D63693048
fbshipit-source-id: 26b1da24db1b6f319af8cef6f62e4739c3f65ee3
* Fix NPE on ReactTextInputManager.setTextDecorationLine (#46750)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46750
Fixes https://github.com/facebook/react-native/issues/39659
Fix is pretty straightforward, parameter is annotated as Nullable, but is accessed with a `.split` call.
This causes a crash when the `textDecorationLine` property is removed (i.e. is null).
Changelog:
[Android] [Fixed] - Fix NPE on ReactTextInputManager.setTextDecorationLine
Reviewed By: cipolleschi
Differential Revision: D63689492
fbshipit-source-id: 3424897cc40beaeb579e3affd0a87656ff43afee
* RNGP - Do not apply `java-gradle-plugin` on root build (#46755)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46755
There is no need to apply `java-gradle-plugin` to the root build of RNGP as that does not contain any Gradle Plugins (they're in subfolders).
This is actually causing the build to be a bit slower as extra tasks for compilation/bundling will be created which are definitely not needed.
Changelog:
[Internal] [Changed] - RNGP - Do not apply `java-gradle-plugin` on root build
Reviewed By: cipolleschi
Differential Revision: D63695934
fbshipit-source-id: 84268105c73b49afdcd94610fcb7006ef53306a4
* Do not reference `setTranslucentBackgroundDrawable` (#46754)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46754
This method has been deprecated since 0.76, but we're still using it inside our codebase.
I'm cleaning up the only usage of it replacing it with the implementation.
Changelog:
[Internal] [Changed] - Do not reference `setTranslucentBackgroundDrawable`
Reviewed By: cipolleschi
Differential Revision: D63695933
fbshipit-source-id: d39b13b584d3ac1406c61c3bf86b86b05dc984d9
* Fix build warning for prepareGlog (#46756)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46756
prepareGlog was firing a warning due to a duplicate config.h file. Here I'm setting so that the file is always overridden (which is the desired behavior) to suppress this warning.
Changelog:
[Internal] [Changed] - Fix build warning for prepareGlog
Reviewed By: cipolleschi
Differential Revision: D63696664
fbshipit-source-id: 83b78afea09c4a5d39f341dd5b604cec466470ae
* Throw JS exception when calling a method if bufferedRuntimeExecutor is not initialized (#46735)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46735
We had some crash on Android where we call [`callFunctionOnModule`](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp#L258) when the `bufferedRuntimeExecutor_` might not be initialized. This can happen when navigating away from RN surface and quickly navigate to another one or across refreshes.
There could be a scheduled JS function from the previous surface/instance that might try to call a native module while the new instance is being created.
This change prevent the crash and replace it with a soft crash, that should show a RedBox on the screen.
## Changelog
[Internal] - Throw JS exception when calling a method if buffereRuntimeExecutor is not initialized
## Facebook
Have a look at T201983945 that generated the crash report
Reviewed By: cortinico
Differential Revision: D63638633
fbshipit-source-id: ba331f5173963265232d0810c2d12895cea3d528
* Fix crash for Modal not attached to window manager (#46758)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46758
There are some occurrencies where the Modal results not attached to the Window, perhaps when the app is backgrounded.
This trigger an exception `java.lang.IllegalArgumentException: View=DecorView@b9f88af[AdsManagerActivity] not attached to window manager`.\
This was fixed already but the conversion from Java to Kotlin missed a condition in the `||` clause. We are adding it back.
## Changelog
[Android][Fixed] - Fix crash for Modal not attached to window manager
## Facebook
The original fix is in this diff D22264672 by mdvacca
Also see this post: https://fb.workplace.com/groups/rn.support/permalink/27047414764880449/
Reviewed By: cortinico
Differential Revision: D63700769
fbshipit-source-id: bc8a44868d5cacb8822e1646c2b7643682f45e1b
* Improve RNTester example for performance.mark/measure (#46759)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46759
Changelog: [internal]
This improves the example in RNTester for `performance.mark` and `performance.measure` by only listing the marks/measures explicitly emitted from the example. This removes noise if other parts of the app are logging marks/measures as well.
Reviewed By: rshest
Differential Revision: D63695731
fbshipit-source-id: f65cf59363da0e6c9e0b3ffadf05abd2eb5a214c
* Replace sh scripts with tested JS scripts to release template (#46363)
Summary:
The previous scripts to trigger the react-native-communty/template
release workflow has not been working. This is a rewrite is js, along
with some testing to make this more robust.
I've have a PR to combine the publish and tag steps in the template publication: https://github.com/react-native-community/template/pull/65, this takes advantage of that change.
Changelog: [Internal]
Pull Request resolved: https://github.com/facebook/react-native/pull/46363
Test Plan:
1. Unit tests
2. Once the infrastructure lands in the `react-native-community/template` workflow, we can trigger a dry run.
## TODO:
- ~~Still needs to be used in the GH release workflow.~~
- ~~Template release workflow needs to land the dry_run input change.~~
## Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D62296008
Pulled By: blakef
fbshipit-source-id: 217326c44b1d820e36a1d847cf9ad24d228087c1
* Bump Metro to 0.81.0-alpha.2 (#46703)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46703
Updates us in line with the release branch and unblocks https://github.com/facebook/react-native/pull/46627.
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63544047
fbshipit-source-id: 4c093e6c2fc389888f12063df013db7023761859
* Add CLI selection of multiple debug targets (#46627)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46627
IMPORTANT: Requires a Metro bump, CI will fail until updated.
Introduces a target selection API for launching React Native DevTools when more than one device is connected.
Credit to robhogan for the initial internal implementation of `OpenDebuggerKeyboardHandler`!
(This leverages recent additions to Metro's reporter API — which we should follow up on to use for the rest of `community-cli-plugin`. Notably, using `TerminalReporter` ensures server output won't conflict with Metro's own event and progress logs.)
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63255295
fbshipit-source-id: da93500358791eabe4cab433cad31b82d518fb5f
* Share TextInputEventEmitter between C++ platforms (#46198)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46198
Extending on https://github.com/facebook/react-native/pull/43431 this change allows to share the TextInputEventEmitter with our C++ based platforms such as RN Windows > https://github.com/microsoft/react-native-windows/blob/main/vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputEventEmitter.h
## Changelog:
[Internal] - Share TextInputEventEmitter between C++ platforms
Reviewed By: NickGerleman
Differential Revision: D61749959
fbshipit-source-id: 2526bf29200cd6a3dc6d5eaef5edbaf4b1df5751
* Cleanup background drawing feature flags (#46761)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46761
The legacy paths for this is all gone and non functional. We can clean thes up fully now.
Changelog: [Internal]
Reviewed By: rshest, Abbondanzo
Differential Revision: D63652264
fbshipit-source-id: cabc1af0098553c64f834e6bc30000ef7a942a4c
* Rename Node.h's getResolvedDimension to getProcessedDimension (#46649)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46649
X-link: https://github.com/facebook/yoga/pull/1705
To get the height and width we call a function currently named `getResolvedDimension`. This returns a `Style::Length`, which in most cases we "resolve" immediately by calling `resolve`. This is a bit confusing that you need to `resolve` something that is already `resolved`.
I plan on adding a new function soon for `contentBox` which would resolve the length for you, so I think this should be renamed.
Also deleted unused `getResolvedDimensions`
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63407730
fbshipit-source-id: e855c17d9c99817be308b7263fcb5d43559ede14
* Clean up calls to dimension() with a hardcoded FlexDirection (#46739)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46739
X-link: https://github.com/facebook/yoga/pull/1710
This is a bit more clear, imo
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63408245
fbshipit-source-id: cea9723914f69d2697e18c8bfe2b55c830af2b6a
* Impl of content box (#46741)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46741
X-link: https://github.com/facebook/yoga/pull/1711
box sizing is really just a reinterpretation of what length properties (like `width`, `height`, `max-width`, etc) mean. So to implement this I just add the border and padding if we are in content box when we ask for any of these properties. All the math that gets done by the algorithm is still in border box land, and the layout we return is to be interpreted as the border box (this is actually the expected behavior per https://drafts.csswg.org/css-sizing/#box-sizing). This makes this implementation pretty simple actually.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63416833
fbshipit-source-id: fd76132cf51e8a5092129802c3a12ab24023018b
* Let flex basis support content box (#46740)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46740
X-link: https://github.com/facebook/yoga/pull/1713
The flex basis length applies to the value of `box-sizing` per the spec: https://drafts.csswg.org/css-flexbox-1/#flex-basis-property
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63424590
fbshipit-source-id: f73bffd30cc7b673453a55089ecc880819d3d788
* Camel case *AxisownerSize (#46744)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46744
X-link: https://github.com/facebook/yoga/pull/1714
This was annoying me :)
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63424778
fbshipit-source-id: 8f9df4e92bde251214f4c1d61ff8282d56f87816
* Plumbing to get boxSizing prop to Yoga (#46766)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46766
tsia!
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D63430964
fbshipit-source-id: f57e9a51236cb4c7d4c284af6987f88f9c7597da
* Add RNTester box sizing example and e2e test (#46765)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46765
Tests are good!
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63471840
fbshipit-source-id: d7bb6f115bcae502e9f8b5ce2a12df8e850d5c26
* Replace execa with child_process (#46767)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46767
Swaps out `execa` dependency for Node's built in `child_process`.
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D63255320
fbshipit-source-id: ce7ef5e2ea78aa96c61815ff8c1e054a8f04e8b0
* Simplify key handling in start command (#46768)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46768
Remove the `KeyPressHandler` util in favour of using the `process.stdin` APIs inline.
This reduces complexity (where we were effectively not using the conditional interception this previously implemented — see also https://github.com/facebook/react-native/pull/46416), and more closely mirrors our internal dev server setup.
Resolves https://github.com/facebook/react-native/issues/46571.
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63255321
fbshipit-source-id: 5cdf7a480053a2215fa50f544733f443bab07ef2
* Replace and remove optional dep on cli-tools logger (#46769)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46769
(Further refactors to logging after D63255296.)
Fully decouples `community-cli-plugin` from the unlisted optional dependency on `react-native-community/cli-tools'`. This is motivated by changes in https://github.com/facebook/react-native/pull/46627 which switch to using Metro's `TerminalReporter` API for emitting logs safely.
- Swaps out logs in the dev server for the `unstable_server_log` Metro reporter event.
- Swaps out `logger.debug()` calls for the `debug` package, currently used by Metro and `dev-middleware`.
- Swaps out other logs in the `bundle` command for `console`.
- (Also specify missing `semver` dep.)
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D63328268
fbshipit-source-id: f552748ecc3456bd5fb8870c3a51d744a6bf3e70
* Tweak start command output (#46770)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46770
Small updates to the `react-native start` command CLI output:
- Change "Welcome..." text to blue.
- Display server base URL instead of port only.
- Embolden key command letters.
- "Ctrl+C to exit" prompt.
- Extra newlines for balance.
Changelog: [Internal]
Reviewed By: cortinico, cipolleschi
Differential Revision: D63328267
fbshipit-source-id: 119ddb2ba68c99df532840285c3c1f922e727e8b
* remove use of type alias from Differentiator (#46762)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46762
changelog: [internal]
Typealias NonOwningList obscures the underlaying type and provides limited benefit. Using std::vector<ShadowViewNodePair*> is more readable.
Reviewed By: NickGerleman
Differential Revision: D63396285
fbshipit-source-id: 70cda3f33a7649cabbf5888fbefe0610d2c002b3
* add support for Android's "High Text Contrast" setting into AccessibilityInfo (#46746)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46746
This change adds `isHighTextContrastEnabled()` to `AccessibilityInfo` to enable access to Android OS's "High contrast text" setting option. It also adds a new event, `highContrastTextChanged`, to enable listeners to subscribe to changes on this setting.
## Changelog
[Android][Added] - Added `isHighTextContrastEnabled()` to `AccessibilityInfo` to read `ACCESSIBILITY_HIGH_TEXT_CONTRAST_ENABLED` setting value
Reviewed By: NickGerleman
Differential Revision: D63155444
fbshipit-source-id: 9829b40e6c183f6beba732190dc318894e9d9a3f
* Define `HostInstance` type for React Native (#46743)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46743
Creates a new `HostInstance` type for React Native, to more accurately capture the intent most developers have when using the `NativeMethods` type or `React.ElementRef<HostComponent<T>>`.
Since `React.ElementRef<HostComponent<T>>` is typed as `React.AbstractComponent<T, NativeMethods>`, that means `React.ElementRef<HostComponent<T>>` is equivalent to `NativeMethods` which is equivalent to `HostInstance`.
Changelog:
[General][Added] - Added `HostInstance` type to represent the instance of a `HostComponent<T>`.
Test Plan:
Ran the following successfully:
```
$ cd ~/fbsource
$ js1 flow
```
Reviewed By: NickGerleman
Differential Revision: D63646555
Pulled By: yungsters
fbshipit-source-id: 075859e6b0217521ba9ab2b92843ae7e47db5be0
* RN: Migrate to `HostInstance` Type (#46742)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46742
Migrates type definitions in React Native to use the newly created `HostInstance` type instead of `NativeMethods` and `React.ElementRef<HostComponent<T>>`.
Changelog:
[General][Changed] - Simplified Flow types to use `HostInstance` (which changing nominal types).
Reviewed By: NickGerleman
Differential Revision: D63646763
fbshipit-source-id: 904894dc40da4d2e70bcb6df47018fc6248ea972
* Animated: Fix `NativeAnimatedModule` Mock for Jest (#46715)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46715
Properly mocks `NativeAnimatedModule` in Jest so that we can cleanup the `process.env.NODE_ENV` checks in Animated.
Changelog:
[General][Added] - Added Jest mocks for `NativeAnimatedModule`
Reviewed By: javache
Differential Revision: D63572067
fbshipit-source-id: a4fcedeebbaaa47a34bb356305652a3ed6358855
* Animated: Change `onEnd` to Private Property (#46721)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46721
Currently, the `Animation` class (from Animated) defines an unofficially protected `__onEnd` property that subclasses are expected to populate. However, subclasses are expected to *not* invoke `__onEnd`, but instead call `__debouncedOnEnd`.
In order to make this code less fragile, this commit refactors the `Animation` class and its subclasses so that `onEnd` is stored in a private property and initialized by the `start` method in the `Animation` parent class. Now, the only way to invoke the `onEnd` callback is by calling `__debouncedOnEnd`.
The subclasses have been adjusted to call the `start` superclass method to initialize this.
Changelog:
[General][Changed] - The `Animation` superclass no longer exposes `__onEnd` as a property. Subclasses must instead invoke `super.start(…)` in their `start()` implementation.
Reviewed By: javache
Differential Revision: D63572063
fbshipit-source-id: 221153ca890c67c59713d607032536f57d7a5b0a
* Animated: Change `nativeID` to Private Property (#46716)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46716
Changes `_nativeId` to be a private property in the `Animation` class (from Animated).
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D63572065
fbshipit-source-id: 24facca0ccb1256f602ec4406bf8c0b22c4bbf96
* Animated: Simplify `TimingAnimation#start` Logic (#46719)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46719
Implements a minor refactor of `TimingAnimation#start` so that we standardize how the `_useNativeDriver` case is handled.
There is no behavior change from this besides more consistently assigning to `this._startTime`. (Previously, we would not set it if the duration were 0 and `!useNativeDriver`.)
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D63572066
fbshipit-source-id: 9cbd05c5ca5e00227be69441b9d7d8a502690246
* Animated: Fail Fast on Native Driver Incompatibility (#46720)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46720
Refactors the `Animation` subclasses (from Animated) such that using `useNativeDriver` with a non-native `AnimatedValue` will consistently fail when calling `start()`.
The current behavior is inconsistent because if a timing or spring animation has a non-zero delay, the error will be thrown in asynchronously in a timeout.
Changelog:
[General][Changed] - Animations started with incompatible `useNativeDriver` and `AnimatedValue` configurations will now synchronously fail. Previously, spring and timing animations with non-zero delays would throw the error asynchronously.
Reviewed By: javache
Differential Revision: D63572062
fbshipit-source-id: 2619e2867fe7754695e3347c838ad6fe136a6614
* Animated: Hoist Common Logic to Parent Class (#46717)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46717
Reduces redundancy and cleans up the layers of abstraction between the `Animation` class and its subclasses, by hoisting common and duplicated logic into the parent class.
I also cleaned up the Flow types in these files while I was at it.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D63572064
fbshipit-source-id: ab6bf7265b67ab38cc5a9d5c94f8cd1456c93f23
* Animated: Rename `__debouncedOnEnd` to `__notifyAnimationEnd` (#46718)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46718
Simple rename of a protected method that should not be called by anyone outside of the `Animation` class and its subclasses, just to make it more intuitive.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D63572363
fbshipit-source-id: cf68f63a9b361767b9dbfcd73124bcdae7a1806e
* Animated: Enqueue Completion Callbacks in Microtask (#46714)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46714
Sets up a feature flag to experiment with scheduling `Animation` completion callbacks in a microtask instead of executing them synchronously. (The completion callback is the callback passed into `animation.start(<callback>)`).
There are currently 4 potential scheduling behaviors of the animation completion callback:
- Synchronously when `start()` is invoked, for spring and timing animations that immediately complete (e.g. no delays and not native driver).
- Synchronously when `stop()` is invoked, if the animation has not yet completed.
- Synchronously during the layout effect phase, or commit phase (if the `useInsertionEffectsForAnimations` feature flag is enabled), if the animation has not yet completed.
- Asynchronously when the animation completes (e.g. non-zero delay or when the native driver is employed).
The wide number of variables and the timing-dependent nature of these variables means that the behavior of the completion callback is very non-deterministic.
Imperically, we have also found that most codebases using `useNativeDriver` mostly have completion callbacks executed asynchronously. However, this leads to surprising problems where the callback causes unpredictable behavior when it is called synchronously (e.g. when the animating component is unmounted *during* the animation).
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D63573322
fbshipit-source-id: 16c48be78c9ac65a64021249fad10f7265737c44
* Small renames in PerformanceEntryReporter (#46688)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46688
Changelog: [internal]
Just aligns on `report*` as the public API in `PerformanceEntryReporter` (e.g.: `measure` -> `reportMeasure`, `logEventEntry` -> `reportEvent`, etc.).
Reviewed By: rshest
Differential Revision: D63471856
fbshipit-source-id: 59617160c238aaefaefe8b6fc27de4481ef32ada
* Remove dependency to PerformanceEntryReporter singleton from RuntimeSchedulerTest (#46760)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46760
Changelog: [internal]
Relying on the singleton was causing issues in iOS tests for RuntimeScheduler. This fixes those issues.
Reviewed By: rshest
Differential Revision: D63702251
fbshipit-source-id: 42b98e673ffba6f45b52510cdb3eea410e55b894
* Refactor Performance and PerformanceObserver internals (#46693)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46693
Changelog: [internal]
This unifies the native modules for `Performance` and `PerformanceObserver` in the same module. Keeping them separate is artificial and introduces unnecessary duplication. These APIs are very closely related so it makes sense to unify
Reviewed By: javache
Differential Revision: D63471855
fbshipit-source-id: fa8c5dc7b7c68954fc11867f68909d2c6c2ee85c
* Optimize locks in PerformanceEntryReporter (#46698)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46698
Changelog: [internal]
Small optimization to use readers/writers for locking in `PerformanceEntryReporter`.
Reviewed By: rshest
Differential Revision: D63540236
fbshipit-source-id: 80b3fd313453ad8da4f0af2deaab61a86064b4b7
* Make detail readonly in PerformanceMark (#46697)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46697
Changelog: [internal]
This is already the case in `PerformanceMeasure` but for some reason we didn't apply it here as well.
Reviewed By: rshest
Differential Revision: D63541134
fbshipit-source-id: 262d504ed153e07e89492945f761887729bad9e0
* Fix Cxx TurboModule methods not being cached (#46751)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46751
rubennorte noticed that some turbo module methods were not getting cached on the jsRepresentation as expected. This is caused by the react-native-codegen wrappers we generate overriding the `get` method and bypassing this caching layer. Instead they should be overriding `create` to lazily allocate new properties. To prevent this from re-occuring, I've made `get` final so no other overrides can happen.
## Changelog:
[General][Fixed] Properties for C++ TurboModules using codegen were not correctly cached
[General][Breaking] TurboModule::get is now a final method, override `create` to customize property lookup
Reviewed By: rubennorte
Differential Revision: D63665966
fbshipit-source-id: c614901c2f0d698147ec9ba36a4b592ed3d37652
* Add missing lock in ReactInstanceManager#attachRootView (#46776)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46776
All other paths that touch `mAttachedReactRoots` uses this lock. We expect this to address a crash we're seeing in production where `startSurface` is invoked multiple times.
Changelog: [Internal]
Reviewed By: fabriziocucci
Differential Revision: D63752083
fbshipit-source-id: f06d07aa70719945f6956fbf6e0cde3e9c8e8ed0
* Rollout destroyFabricSurfacesInReactInstanceManager (#46778)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46778
This has been rolled out internally already and is a good cleanup for OSS too. We no longer use this config externally (BRIDGE+FABRIC) so this shouldn't actually affect the new arch rollout.
Changelog: [Internal]
Reviewed By: fabriziocucci
Differential Revision: D63752084
fbshipit-source-id: da3025f7b072dc7b82f8de22ff247b3979ae64d0
* Add changelog for 0.74.6 (#46783)
Summary:
Add changelog for 0.74.6
## Changelog:
[Internal] - Add changelog for 0.74.6
Pull Request resolved: https://github.com/facebook/react-native/pull/46783
Test Plan: N/A
Reviewed By: cortinico
Differential Revision: D63756879
Pulled By: cipolleschi
fbshipit-source-id: c36fae089c58fad49f45fa3b3380c38490d67293
* prevent C++ state update in RCTScrollViewComponentView if value is the same (#46782)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46782
changelog: [internal]
Avoid setting C++ state for the same contentOffset. This is redundant work and can be safely avoided.
Reviewed By: NickGerleman
Differential Revision: D63700308
fbshipit-source-id: d6ea8cf6a1da47a13787b49abd66a87e3da57d34
* Update modern inspector targets to directly send CDP title and description (#46780)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46780
This is primarily a debugger server change to better-align the `title` and `description` fields (visible in via the CDP `/json/list` endpoint), reporting them directly from the device.
For React Native users, the net effect of this change is to improve/align the display name for each debugger target in the CLI multi-select menu (`j` to debug). This change may also be useful for discovery in non-Fusebox frontends such as VS Code (`vscode-expo` extension).
Changes:
- Rename `title` prop on `InspectorPageDescription`/`IInspector::addPage` to `description` (no call site changes).
- Add `deviceName` param to `InspectorPackagerConnection`.
- Move the page `description` to the `description` JSON field.
- Update `InspectorPackagerConnection::Impl::pages()` to return new `title` and `description` fields.
- Align `OpenDebuggerKeyboardHandler` to display `title` field.
- Deprecate the nonstandard `deviceName` field.
**Before**
```
[
{
"id": "3c9f24bedab0e73fca6a1b295030e7af9346a8c0-1",
"title": "React Native Bridgeless [C++ connection]",
"description": "com.facebook.RNTester",
...
}
```
The `description` field was previously the closest thing we had to a target identifier. Today, this is not needed, since we have the stable `reactNative.logicalDeviceId` field.
**After**
```
[
{
"id": "3c9f24bedab0e73fca6a1b295030e7af9346a8c0-1",
"title": "com.facebook.RNTester (iPhone 16 Pro)",
"description": "React Native Bridgeless [C++ connection]",
...
}
```
The `title` field is now more human readable and aligned with what we render in the window title of React Native DevTools. The `description` field describes the type of debugger target (the specific React Native implementation).
Changelog: [Internal]
Reviewed By: vzaidman
Differential Revision: D63329456
fbshipit-source-id: cfe98f77e31c729431005925cfc66e2780ef8c72
* make sure logs reflect new class name (#46785)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46785
changelog: [internal]
Make sure logs match the class name
Reviewed By: rubennorte
Differential Revision: D63757693
fbshipit-source-id: 815e70e8ae38d99b1908d3aa2a9cfffdd9dc9851
* Fix reported properties for native modules (#46788)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46788
Changelog: [internal]
This fixes the reported keys for C++ TurboModules:
```
import MyNativeCppModule from 'MyNativeCppModule';
Object.keys(MyNativeCppModule); // [] - expected
Object.keys(Object.getPrototypeOf(MyNativeCppModule)); // [] - NOT expected
```
Before, we were trying to read the properties from the method map in the public `TurboModule`, but in this implementation all the logic is actually in its delegate (also a `TurboModule` instance, yeah, confusing).
This fixes the issue by forwarding the call to the delegate that has the right information.
Reviewed By: javache
Differential Revision: D63761381
fbshipit-source-id: 90bd216efa0f60352c5ca341d210d83239c80dba
* Create ReactPerfLogger to log to Perfetto and Fusebox (#46793)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46793
Currently performance logging has several problems:
- We have widly different APIs. It's hard to know which to use for what.
- Most of these APIs don't allow retroactive timestamp which is important for hero logging and react flame graphs
- These APIs aren't accessible from JS/C++/Java/Koltin (this part will be fixed in another diff)
- Logging doesn't go to the right place. It either only goes to Systrace, or Perfetto but with broken async timeline etc...
This diff start addressing the problem by refactoring. Currently performance.measure/performance.mark are the best APIs. Let's start with refactoring them so that we can call them directly without the otherwise PerformanceEntry side effects.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D63560473
fbshipit-source-id: 86755b3e8491a7cf6919173a07ba9421cd30e663
* Fix onEndReached not being called when getItemLayout is present and we scroll past render window (#46736)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46736
Copying the comment in code:
> We only call `onEndReached` after we render the last cell, but when getItemLayout is present, we can scroll past the last rendered cell, and never trigger a new layout or bounds change, so we need to check again after rendering more cells.
Changelog:
[General][Fixed] - Fix onEndReached not being called when getItemLayout is present and we scroll past render window
Reviewed By: yungsters
Differential Revision: D63643856
fbshipit-source-id: 9c53789cb15b225ceac353c37cbb67f7beeaf4fb
* Back out "Add RNTester box sizing example and e2e test" (#46795)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46795
Original commit changeset: d7bb6f115bca
Original Phabricator Diff: D63471840
See previous diff, there were some problems
Changelog: [Internal]
Differential Revision: D63777438
fbshipit-source-id: ea9f370eade10282fcee84d07d709b549dde1b6d
* Back out "Plumbing to get boxSizing prop to Yoga" (#46794)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46794
Original commit changeset: f57e9a51236c
Original Phabricator Diff: D63430964
Seeing some issues with this so gonna turn it off for the meantime.
Changelog: [Internal]
Differential Revision: D63777437
fbshipit-source-id: fe8abd0110b2cf7296911c5b1e95ada40d30994c
* Remove run on iOS/Android key handlers (#46781)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46781
Implements https://github.com/react-native-community/discussions-and-proposals/discussions/821.
Changelog:
[General][Removed] - Remove "run on iOS" and "run on Android" from the dev server key commands
Reviewed By: cortinico
Differential Revision: D63534248
fbshipit-source-id: 003d4c639bbf9128f921936508a62de2caadac5e
* Unbreak build_android (#46806)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46806
This fixes the CI which is currently red due to wrong C++ imports.
Changelog:
[Internal] [Changed] - Unbreak build_android
Reviewed By: javache, cipolleschi
Differential Revision: D63826953
fbshipit-source-id: 4d9e407331af666a420cf56178420df40e598ef4
* chore: Add changelog for 0.75.4 (#46792)
Summary:
Adds changelog for the 0.75.4 patch.
## Changelog:
[Internal] [Changed] - Add 0.75.4 changelog
Pull Request resolved: https://github.com/facebook/react-native/pull/46792
Reviewed By: blakef
Differential Revision: D63766181
Pulled By: cipolleschi
fbshipit-source-id: f1b4e9e0676bc7dd32fe9d86386552f273fdcc75
* Rename `RCTUIGraphicsImageRenderer` to `RCTMakeUIGraphicsImageRenderer` (#46772)
Summary:
Because `UIGraphicsImageRenderer` doesn't exist on macOS, I need to shim it for React Native macOS (See https://github.com/microsoft/react-native-macos/pull/2209). I planned to use the name `RCTUIGraphicsImageRenderer`. However.. it seems that is used by a static helper function in `RCTBorderDrawing.m`. So.. let's rename it? The function is just a helper method to make an instance of the class, so I think the name `RCTMakeUIGraphicsImageRenderer` is slightly more idiomatic anyway.
This method is not public, so it should not break the public API of React Native.
## Changelog:
[IOS] [CHANGED] - Rename `RCTUIGraphicsImageRenderer` to `RCTMakeUIGraphicsImageRenderer`
Pull Request resolved: https://github.com/facebook/react-native/pull/46772
Test Plan: CI should pass
Reviewed By: joevilches
Differential Revision: D63765490
Pulled By: cipolleschi
fbshipit-source-id: de68dce0f92ec249ea8586dbf7b9ba34a8476074
* Allow taking control of bundle loading on new arch (#46731)
Summary:
On the old architecture you could take control of loading the bundle by implementing
```objc
- (void)loadSourceForBridge:(RCTBridge *)bridge
onProgress:(RCTSourceLoadProgressBlock)onProgress
onComplete:(RCTSourceLoadBlock)loadCallback;
```
in your `RCTBridgeDelegate`. This is not currently possible in the new architecture.
I've added this using a pretty much identical api by adding a function to both the `RCTInstanceDelegate` and `RCTHostDelegate` protocols. This will be called on the `RCTRootViewFactory`. I've added two properties to the `RCTRootViewFactoryConfiguration`, `loadSourceForHost` and `loadSourceWithProgressForHost`. If one is present, we call it, otherwise we fallback to the normal loading process
## Changelog:
[iOS] [Breaking] - Add ability to control bundle loading on the new architecture similar to `loadSourceForBridge`. Removed some properties from the `RCTRootViewFactory`.
Pull Request resolved: https://github.com/facebook/react-native/pull/46731
Test Plan: Rn-tester works as normal and it is working for our use case in expo go.
Reviewed By: blakef
Differential Revision: D63755188
Pulled By: cipolleschi
fbshipit-source-id: f1f26b2775b9e547ce7a23028665797c1…
blakef
pushed a commit
that referenced
this pull request
Oct 30, 2024
Summary: Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Pull Request resolved: #46637 Reviewed By: huntie Differential Revision: D63349613 Pulled By: rickhanlonii fbshipit-source-id: 32e3fa4e2f2077114a6e9f4feac73673973ab50c
cipolleschi
pushed a commit
that referenced
this pull request
Oct 31, 2024
* Re-enable integration tests (#46639) Summary: Pull Request resolved: #46639 These tests were skipped when we were switching to component stacks, which also hid a bug later in the stack. Re-enable them. Changelog: [Internal] Reviewed By: javache Differential Revision: D63349616 fbshipit-source-id: ccde7d5bb3fcd9a27adf4af2068a160f02f7432a * Add integration tests for console errors + ExceptionManager (#46636) Summary: Pull Request resolved: #46636 Adds more integration tests for LogBox (currently incorrect, but fixed in a later diff). Changelog: [Internal] Reviewed By: javache Differential Revision: D63349614 fbshipit-source-id: 8f5c6545b48a1ed18aea08d4ecbecd7a6b9fa05a * Refactor LogBox tests to spies (#46638) Summary: Pull Request resolved: #46638 This is annoying, but in the next diff that fixes a bug I need to test using the default warning filter instead of a mock (really, all this mocking is terrible, idk why I did it this way). Unfortunately, in Jest you can't just reset mocks from `jest.mock`, `restoreMocks` only resets spies and not mocks (wild right). So in this diff I converted all the `jest.mock` calls to `jest.spyOn`. I also corrected some of the mocks that require `monitorEvent: 'warning',` like the warning filter sets. I also added a test that works without the fix. Changelog: [Internal] Reviewed By: javache Differential Revision: D63349615 fbshipit-source-id: 4f2a5a8800c8fe1a10e3613d3c2d0ed02fca773e * Fix errors with component stacks reported as warnings (#46637) Summary: Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Pull Request resolved: #46637 Reviewed By: huntie Differential Revision: D63349613 Pulled By: rickhanlonii fbshipit-source-id: 32e3fa4e2f2077114a6e9f4feac73673973ab50c * [LOCAL] using older version of React Dev Tools - Older version has old URL, updated tests - Comments on test don't match what's being tested. Updated. --------- Co-authored-by: Rick Hanlon <rickhanlonii@meta.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Ok so this is a doozy.
Overview
There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally).
However, in when I switched from using the
Warning:prefix, to using the presence of component stacks, I subtly missed the default warning filter case.In the internal warning filter, the
monitorEventis always set to something other thanunknownand if it's set towarning_unhandledthensuppressDialog_LEGACYis always false.However, the default values for the warning filter are that
monitorEvent = 'unknown'andsuppressDialog_LEGACY = true. In this case, we would downlevel the error to a warning.What's the fix?
Change the default settings for the warning filter.
What's the root cause?
Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests)
How could it have been caught?
It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on.
Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings
Differential Revision: D63349613