Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge latest upstream RN 0.71 changes #1868

Merged
merged 48 commits into from
Jul 6, 2023

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Jul 5, 2023

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Pick up the latest few patch releases of upstream React Native 0.71 into our fork. In particular, I want this change related to JSC: facebook#37501 that fixes this Xcode 14.3 issue: facebook#36794

Changelog

[GENERAL] [ADDED] - Merge latest RN 0.71 changes

Test Plan

CI passes

kelset and others added 30 commits April 11, 2023 12:16
Summary:
D42721684 (facebook@be69c8b) left a pretty bad bug when using Fabric for Android. I missed that in Fabric specifically, on edit we will cache the Spannable backing the EditText for use in future measurement.

Because we've stripped the sizing spans, Spannable measurement has incorrect font size, and the TextInput size will change (collapsing) after the first edit. This effectively breaks any uncontrolled TextInput which does not have explicit dimensions set.

Changelog:
[Android][Fixed] - Fix measurement of uncontrolled TextInput after edit

Reviewed By: sammy-SC

Differential Revision: D43158407

fbshipit-source-id: 51602eab06c9a50e2b60ef0ed87bdb4df025e51e
Summary:
Pull Request resolved: facebook#36543

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

We cache the backing EditText span on text change to later measure. To measure outside of a TextInput we need to restore any spans we removed. Spans may overlap, so base attributes should be behind everything else.

The logic here for dealing with precedence is incorrect, and we should instead accomplish this by twiddling with the `SPAN_PRIORITY` bits.

Changelog:
[Android][Fixed] - Minimize Spans 1/N: Fix precedence

Reviewed By: javache

Differential Revision: D44240779

fbshipit-source-id: f731b353587888faad946b8cf1e868095cdeced3
…ic (facebook#36546)

Summary:
Pull Request resolved: facebook#36546

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This change generalizes `stripAttributeEquivalentSpans()` to allow plugging in different spans.

Changelog:
[Internal]

Reviewed By: rshest

Differential Revision: D44240781

fbshipit-source-id: 89005266020f216368e9ad9ce382699bd8db85a8

# Conflicts:
#	ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java
Summary:
Pull Request resolved: facebook#36547

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This adds `ReactBackgroundColorSpan` to the list of spans eligible to be stripped.

Changelog:
[Android][Fixed] - Minimize Spans 3/N: ReactBackgroundColorSpan

Reviewed By: javache

Differential Revision: D44240782

fbshipit-source-id: 2ded1a1687a41cf6d5f83e89ffadd2d932089969
Summary:
Pull Request resolved: facebook#36545

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This adds ReactForegroundColorSpan to the list of spans eligible to be stripped.

Changelog:
[Android][Fixed] - Minimize Spans 4/N: ReactForegroundColorSpan

Reviewed By: javache

Differential Revision: D44240780

fbshipit-source-id: d86939cc2d7ed9116a4167026c7d48928fc51757
)

Summary:
Pull Request resolved: facebook#36544

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This change makes us apply strikethrough and underline as paint flags to the underlying EditText, instead of just the spans. We then opt ReactUnderlineSpan and ReactStrikethroughSpan into being strippable.

This does actually create visual behavior changes, where child text will inherit any underline or strikethrough of the root EditText (including if the child specifies `textDecorationLine: "none"`. The new behavior is consistent with both iOS and web though, so it seems like more of a bugfix than a regression.

Changelog:
[Android][Fixed] - Minimize Spans 5/N: Strikethrough and Underline

Reviewed By: rshest

Differential Revision: D44240778

fbshipit-source-id: d564dfc0121057a5e3b09bb71b8f5662e28be17e
Summary:
Pull Request resolved: facebook#36548

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This change lets us set `letterSpacing` on the EditText instead of using our custom span.

Changelog:
[Android][Fixed] - Minimize EditText Spans 6/N: letterSpacing

Reviewed By: rshest

Differential Revision: D44240777

fbshipit-source-id: 9bd10c3261257037d8cacf37971011aaa94d1a77
Summary:
Pull Request resolved: facebook#36576

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This change addresses some minor CR feedback and removes the temporary list of spans in favor of applying them directly.

Changelog:
[Internal]

Reviewed By: javache

Differential Revision: D44295190

fbshipit-source-id: bd784e2c514301d45d0bacd8ee6de5c512fc565c
Summary:
Pull Request resolved: facebook#36577

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

This change allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well.

Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here.

Changelog:
[Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan

Reviewed By: javache

Differential Revision: D44297384

fbshipit-source-id: ed4c000e961dd456a2a8f4397e27c23a87defb6e
…k#36575)

Summary:
Pull Request resolved: facebook#36575

This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( facebook#35936 (comment)) for greater context on the platform behavior.

D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all.

We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding.

Changelog:
[Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()`

Reviewed By: javache

Differential Revision: D44298159

fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002

# Conflicts:
#	ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
Summary:
We moved Hermes some build utils from [Hermes repo](https://github.com/facebook/hermes/tree/main/utils) to [React Navtie repo](https://github.com/facebook/react-native/tree/main/sdks/hermes-engine/utils) a while ago to have more control over them. However some paths on the CI were not updated. We continued to use old build scripts for Hermes prebuilds. Some unfortunate side effects are:
- `HERMES_ENABLE_DEBUGGER` is [hardcoded to true](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L65). That makes Hermes much slower in Release configuration.
- BUILD_TYPE is [set to Release](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L10) instead of `MinSizeRel` which inreases Hermes binary size.

This diff copies these build utils from RN to Hermes source directory before we perform Hermes build.

Changelog
[Internal]

Reviewed By: cipolleschi

Differential Revision: D44066721

fbshipit-source-id: f45ad6a31fb01c10199f69cc7bbcbbc83b793d34
Summary: Changelog: [Android][Fixed] Resolved bug with Text components in new arch losing text alignment state.

Reviewed By: mdvacca

Differential Revision: D34108943

fbshipit-source-id: 3992e9406345be919b5e3595fc1f9e61cf67a699

# Conflicts:
#	ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java
#	ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
…tView (facebook#36930)

Summary:
Pull Request resolved: facebook#36930

This diff fixes facebook#36494

Now this code matches its [Fabric counterpart](facebook@7b48899).

There is also one extra check that `_lastStringStateWasUpdatedWith != nil`. With that in place we don't send extra `onChangeText` event when `attributedText` is assigned for the first time on TextView construction.

Changelog: [IOS][Fixed] - Do not send extra onChangeText even wnen instantianting multiline TextView

Reviewed By: sammy-SC

Differential Revision: D45049135

fbshipit-source-id: 62fa281308b9d2e0a807d024f08d8a214bd99b5e
Summary:
The [React Native TV repo](https://github.com/react-native-tvos/react-native-tvos) shares most of the same Android code as the core repo. Beginning in 0.71, it needs to also publish Android Maven artifacts for the `react-android` and `hermes-android` libraries.

In order to avoid conflicts, it needs to publish the artifacts to a different group name. However, `react-native-gradle-plugin` uses a hardcoded group name (`com.facebook.react`).

Solution: read the group name from the existing `GROUP` property in `ReactAndroid/gradle.properties`.

## Changelog:

[Android] [Fixed] - read GROUP name in gradle-plugin dependency code

Pull Request resolved: facebook#37204

Test Plan:
- Android unit tests have been added for the new code and new method in `DependencyUtils.kt`.
- Existing tests should pass
- The new code defaults to the correct group (`com.facebook.react`) so no functional change is expected in the core repo.

Reviewed By: luluwu2032

Differential Revision: D45576700

Pulled By: cortinico

fbshipit-source-id: 6297ab515b4bdbb17024989c7d3035b0a2ded0ae

# Conflicts:
#	packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt
#publish-packages-to-npm
Summary:
Since v0.71 the autoComplete prop on TextInput is available on iOS ([release notes](https://reactnative.dev/blog/2023/01/12/version-071#component-specific-behavior)). However, this change is not reflected in the types.

Original types PR here - DefinitelyTyped/DefinitelyTyped#65144 by chwallen

## Changelog:

[GENERAL] [FIXED] - Fix autoComplete type for TextInput

Pull Request resolved: facebook#36931

Test Plan: Setting the autoComplete prop on TextInput to `nickname`, `organization`, `organization-title`, or `url` should not result in typescript errors.

Reviewed By: NickGerleman

Differential Revision: D45052350

Pulled By: javache

fbshipit-source-id: 40993833b4ed14f91e3bf3521a264ea93517a0c9
Summary:
Pull Request resolved: facebook#37569

Fixes facebook#37543

Missed as part of D39423409 (or maybe we didn't have TS types inline yet?)

Changelog:
[General][Fixed] - Allow string `transform` style in TypeScript

Reviewed By: cortinico

Differential Revision: D46161450

fbshipit-source-id: 24ee9e19365b7209ec0a2c8fb5a5d7ac78203f4d
…37445)

Summary:
For 3rd party libraries to work with a React Native fork (such as the TV repo) that uses a different Maven group for `react-android` and `hermes-android` artifacts, an additional dependency substitution is required.

## Changelog:

[Android][fixed] RNGP dependency substitutions for fork with different Maven group

Pull Request resolved: facebook#37445

Test Plan:
- Manual tested with an existing project
- Unit tests pass

Reviewed By: rshest, dmytrorykun

Differential Revision: D45948901

Pulled By: cortinico

fbshipit-source-id: 4151a1d3616172a92c68812c3a0034c98b330d67
Summary:
Fixes facebook#36066

## Changelog

[GENERAL] [FIXED] - VirtualizedList scrollToEnd with no data

Pull Request resolved: facebook#36067

Test Plan: Run `yarn test VirtualizedList-test`

Reviewed By: jacdebug

Differential Revision: D43041763

Pulled By: javache

fbshipit-source-id: d4d5e871284708a89bf9911d82e9aa97d7625aca
Summary:
Pull Request resolved: facebook#37452

Fixes facebook#37428

We do have cache poisoning for hermesc on Windows and Linux due to reusing the same cache key among different
React Native version. This fixes it by specifying a cache key which is version dependent + it invalidates the
caches by defining a new key.

Changelog:
[Internal] [Fixed] - Make CircleCI caches for hermesc be version dependent

Reviewed By: cortinico

Differential Revision: D45909178

fbshipit-source-id: 830c87ae45739c7053342a68dac2ee7581945c1d
#publish-packages-to-npm
Distiller and others added 18 commits June 7, 2023 17:15
Summary:
Hi 👋

I'm one of the  Expo contributors. When upgrading the Stripe dependency in Expo for SDK48 release on Android, we noticed the following error:
```java.lang.ClassCastException: android.view.ContextThemeWrapper cannot be cast to android.app.Activity```

With Kudo we narrowed it down to an underlying issue in the following cast in `ReactRootView` done during checking for keyboard events:
```((Activity) getContext())```

The `getContext()` is actually a `ContextThemeWrapper` for `ExperienceActivity`, so we should not cast it to an `Activity` directly - instead, we unwrap it using `getBaseContext()`.

Implementing the following fix into the Expo fork fixed the crash for us:
expo@0e2c9ca

## Changelog

[ANDROID] [FIXED] - Fixed crash occurring in certain native views when handling keyboard events.

<!-- 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: facebook#36121

Test Plan:
Tested manually by comparing two builds before and after this change. The main branch build crashes when the [Stripe 0.23.1](https://github.com/stripe/stripe-react-native) Card Element from the example app is mounted on screen. Applying the change fixed the issue.

Happy to make a more isolated reproduction if necessary.

 ---
Full stack trace:
```
         AndroidRuntime  D  Shutting down VM
                         E  FATAL EXCEPTION: main
                         E  Process: host.exp.exponent, PID: 8849
                         E  java.lang.ClassCastException: android.view.ContextThemeWrapper cannot be cast to android.app.Activity
                         E      at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:937)
                         E      at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:913)
                         E      at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061)
                         E      at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3352)
                         E      at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2286)
                         E      at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8948)
                         E      at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1231)
                         E      at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239)
                         E      at android.view.Choreographer.doCallbacks(Choreographer.java:899)
                         E      at android.view.Choreographer.doFrame(Choreographer.java:832)
                         E      at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214)
                         E      at android.os.Handler.handleCallback(Handler.java:942)
                         E      at android.os.Handler.dispatchMessage(Handler.java:99)
                         E      at android.os.Looper.loopOnce(Looper.java:201)
                         E      at android.os.Looper.loop(Looper.java:288)
                         E      at android.app.ActivityThread.main(ActivityThread.java:7898)
                         E      at java.lang.reflect.Method.invoke(Native Method)
                         E      at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                         E      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
```

Reviewed By: cortinico

Differential Revision: D43186530

Pulled By: javache

fbshipit-source-id: 2143495f6b0c71f342eba6d5abb2bfa4a529fbdd
…ok#37487)

Summary:
We've been encountering a crash in `runAnimationStep` with "Calculated frame index should never be lower than 0" facebook#35766 with OnePlus/Oppo devices as well, but don't have one on hand to test.

This just works around the issue: if the time is before the start time of an animation, we shouldn't do anything anyways, so we just log a message instead of throwing while in production. We still throw in debug mode though for easier debugging.

### Hypothesis of the root cause

Based on stacktrace in facebook#35766 (which is the same one we see)

Normally, this should happen

1. Choreographer.java constructs a FrameDisplayEventReceiver
2. FrameDisplayEventReceiver.onVSync gets called, which sets the `mTimestampNanos`
3. FrameDisplayEventReceiver.run gets called, which then eventually calls our `doFrame` callback with `mTimestampNanos`. This then causes `FrameBasedAnimationDriver.runAnimationStep` to be called with the same timestamp

I suspect what's happening on OnePlus devices is that the `onVSync` call either doesn't happen or happens rarely enough that the `mTimestampNanos` when `run` is called is sometime in the past

### Fix

1. Add logging so we get the parameters to debug more if we end up getting this error
2. In production, just ignore past times instead of throwing an Error

## Changelog:

Pick one each for the category and type tags:

[ANDROID] [FIXED] - Prevent crash on OnePlus/Oppo devices in runAnimationStep

Pull Request resolved: facebook#37487

Test Plan: Ran our app using patched version and verified no issues showed up when using it

Reviewed By: cipolleschi

Differential Revision: D46102968

Pulled By: cortinico

fbshipit-source-id: bcb36a0c2aed0afdb8e7e68b141a3db4eb02695a
…cebook#37501)

Summary:
Pull Request resolved: facebook#37501

This is the iOS side of the fix for facebook#36794.

That issue aside for the moment, the high-level idea here is to conceptually separate the bundle *request URL*, which represents a request for the *latest* bundle, from the *source URL* passed to JS engines, which should represent the code actually being executed. In future, we'd like to use this to refer to a point-in-time snapshot of the bundle, so that stack traces more often refer to the code that was actually run, even if it's since been updated on disk (actually implementing this isn't planned at the moment, but it helps describe the distinction).

Short term, this separation gives us a way to address the issue with JSC on iOS 16.4 by allowing Metro to provide the client with a [JSC-safe URL](react-native-community/discussions-and-proposals#646) to pass to the JS engine, even where the request URL isn't JSC-safe.

We'll deliver that URL to the client on HTTP bundle requests via the [`Content-Location`](https://www.rfc-editor.org/rfc/rfc9110#name-content-location) header, which is a published standard for communicating a location for the content provided in a successful response (typically used to provide a direct URL to an asset after content negotiation, but I think it fits here too).

For the long-term goal we should follow up with the same functionality on Android and out-of-tree platforms, but it's non-essential for anything other than iOS 16.4 at the moment.

For the issue fix to work end-to-end we'll also need to update Metro, but the two pieces are decoupled and non-breaking so it doesn't matter which lands first.

Changelog:
[iOS][Changed] Prefer `Content-Location` header in bundle response as JS source URL

Reviewed By: huntie

Differential Revision: D45950661

fbshipit-source-id: 170fcd63a098f81bdcba55ebde0cf3569dceb88d
Summary:
Adds compat with W3C logical CSS properties. See facebook#34425

This is a replacement for reverted facebook#34590, which can no longer be imported internally.

[General][Added] - Added CSS logical properties.

Pull Request resolved: facebook#35316

Test Plan: Unit test snapshots.

Reviewed By: NickGerleman

Differential Revision: D41230978

Pulled By: necolas

fbshipit-source-id: 40e93d0d697f0cb28390480ce2b4bcbce18da70a
…ick_logical-style-props-and-tests

Add web compat unit tests and fixes
Summary:
Pull Request resolved: facebook#38005

Fixes facebook#32093 by guarding the expensive `BABEL_CODE_FRAME_ERROR_FORMAT` regex with a cheaper initial scan. (Longer term, we should reduce our reliance on string parsing and propagate more structured errors.)

Changelog: [General][Fixed] Prevent LogBox from crashing on very long messages

Reviewed By: GijsWeterings

Differential Revision: D46892454

fbshipit-source-id: 3afadcdd75969c2589bbb06f47d1c4c1c2690abd

# Conflicts:
#	Libraries/LogBox/Data/parseLogBoxLog.js
#	packages/react-native/package.json
@Saadnajmi Saadnajmi requested a review from a team as a code owner July 5, 2023 16:04
@Saadnajmi Saadnajmi merged commit 8a76239 into microsoft:0.71-stable Jul 6, 2023
14 of 16 checks passed
@Saadnajmi Saadnajmi deleted the more-71 branch July 6, 2023 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet