Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37501
This is the iOS side of the fix for https://github.com/facebook/react-native/issues/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](https://github.com/react-native-community/discussions-and-proposals/pull/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:
Changelog: [Internal][iOS] Minor: Rename RCTNotAllowedInAppWideFabric to RCTNotAllowedInFabricWithoutLegacy
`RCTNewArchitectureValidationPlaceholder(RCTNotAllowedInBridgeless` is to track Bridge APIs that are okay in Fabric but not in Bridgeless.
`RCTNewArchitectureValidationPlaceholder(RCTNotAllowedInFabricWithoutLegacy` is to track legacy APIs that should not exist if the app was using Fabric **without any legacy architecture**. e.g. RCTBridgeModule, legacy interop view components.
Reviewed By: fkgozali
Differential Revision: D37659105
fbshipit-source-id: aee4e083820e83a8dac19eb3b5efc49b37d90039
Summary:
Changelog: [Internal]
- Make the new architecture validation easier to understand by enabling validation with `RCTNewArchitectureSetMinValidationLevel(level)`.
- When `RCT_ONLY_NEW_ARCHITECTURE` flag is enabled:
- `RCTErrorNewArchitectureValidation` calls `RCTLogAssert` instead of `RCTLogError`.
- `RCTNewArchitectureValidationPlaceholder` calls `RCTLog`, instead of no-op.
Reviewed By: fkgozali
Differential Revision: D37555667
fbshipit-source-id: 2c725c287a2dec19e8946c7fe5d8fa111e4a17fa
Summary:
This fix solves a problem very well evaluated [here](https://github.com/Expensify/App/issues/2727) as well as this [one](https://github.com/facebook/react-native/issues/29290).
The issue is that when the app goes to background, in landscape mode, the RCTDeviceInfo code triggers an orientation change event that did not physically happen. Due to that, we get swapped values returned when going back to the app.
I debugged the react-native code, and to me it seems that react native publishes the orientation change event one extra time when switching the state of the app to 'inactive'. Here is what is happening:
1. iPad is in landscape.
2. We move the app to inactive state.
3. Native Code queues portrait orientation change (no such change happened physically), and immediately after it triggers landscape change (same as we had in point 1).
4. We restore the app to active state.
5. The app receives two queued orientation change events, one after another.
6. The quick transition between portrait and landscape happens even though it never went back to portrait.
Fresh `react-native init` app repro case can be found here: https://github.com/lbaldy/issue-34014-repro
Video presenting the issue (recorded while working on: https://github.com/Expensify/App/issues/2727 ): https://www.youtube.com/watch?v=nFDOml9M8w4
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Fixed] - Fix the way the orientation events are published, to avoid false publish on orientation change when app changes state to inactive
Pull Request resolved: https://github.com/facebook/react-native/pull/34014
Test Plan:
### Test Preparation
1. Make sure you have a working version of E/App.
2. Open App/src/components/withWindowDimensions.js and update the constructor by changing this line:
`this.onDimensionChange = _.debounce(this.onDimensionChange.bind(this), 100);`
to
`this.onDimensionChange = this.onDimensionChange.bind(this);`
3. Open the NewExpensify.xcodeproj in xCode.
4. Open the RCTDeviceInfo.mm file and replace it's contents with the file from this PR.
5. Select your device of choice (I suggest starting with iPad mini) and run the app though xCode.
6. From this point you can move to the test scenarios described below.
### iPad Mini tests:
Reproduction + Fix test video (Test 1): https://youtu.be/jyzoNHLYHPo
Reproduction + Fix test video (Test 2): https://youtu.be/CLimE-Fba-g
**Test 1:**
1. Launch app in portrait, open chat - no sidebar visible.
7. Switch to landscape - sidebar shows.
8. Put app to background.
9. Put app back to foreground - make sure the side menu doesn't flicker.
**Test 2:**
1. Launch app in portrait, open chat - no sidebar visible.
2. Switch to landscape - sidebar shows.
3. Put app to background. Switch orientation back to portrait.
4. Put app back to foreground - make sure the side menu hides again as it should be in portrait.
### iPad Pro tests:
Reproduction + Fix test video (Test 3, Test 4): https://youtu.be/EJkUUQCiLRg
iPad mini test 1 applies.
Scenario 2 does not as the screen is too wide in both orientations and iPad pro shows sidebar always.
**Test 3:**
1. launch the app.
2. Make sure you're in landscape mode.
3. See split screen with some other app. Make sure the side bar is visible.
4. Play with the size of the view, resize it a bit. When the view shrinks it should hide the sidebar, when it grows it should show it.
10. Move the app to background and back to foreground, please observe there are no flickers.
**Test 4:**
1. Launch the app.
2. Make sure you're in landscape mode.
3. Make the multitasking view and make Expensify app a slide over app.
4. Move back to fullscreen/split screen. Make sure the menu is shown accordingly
5. Move the app to background and back to foreground, please observe there are no flickers.
### iPhone:
Non reg with and without the fix video: https://youtu.be/kuv9in8vtbk
Please perform standard smoke tests on transformation changes.
Reviewed By: cipolleschi
Differential Revision: D37239891
Pulled By: jacdebug
fbshipit-source-id: e6090153820e921dcfb0d823e0377abd25225bdf
Summary:
Original commit changeset: 78387999f94e
Original Phabricator Diff: D34392529 (https://github.com/facebook/react-native/commit/086c13dd5fba3f77acbc70c9bdedc9299118b526)
Backing this out because it breaks univeral hot reload support. We should probably find a way to support this *without* relying on swizzling. This was originally backed out it because it was blocking app store submission, but this is gated by `RN_DEV` so should never be included in a release build.
Changelog:
[General][Removed] - The diffs renames the required variable which was causing conflicts in names with Apple core SDK's
Reviewed By: cipolleschi
Differential Revision: D37311377
fbshipit-source-id: 18abb1b53a5be054098cd3717705ea5086c4f595
Summary:
# Context
ScrollView sticky headers rely on this bit of code to work:
```
AnimatedImplementation.attachNativeEvent(
this._scrollViewRef,
'onScroll',
[{nativeEvent: {contentOffset: {y: this._scrollAnimatedValue}}}],
);
```
What this code means:
When the ScrollView host component receives the "onScroll" event, assign event.nativeEvent.contentOffSet.y to the this._scrollAnimatedValue AnimatedValue.
How this subscription mechanism is set up:
NativeAnimatedTurboModule subscribes to events dispatched by RCTEventDispatcher sendEvent. Then, whenever RCTEventEmitter sendEvent executes, NativeAnimatedTurboModule also updates the AnimatedValue for that event.
# Problem
Previously, in bridgeless, we started dispatching RCTScrollView via the RCTEventDispatcher sendEvent to update the AnimatedValue for ScrollView. This meant that we started dispatching the onScroll event to JavaScript via RCTEventEmitter.receiveEvent JSModule, which isn't supported in the Fabric renderer.
With this diff, we dialed back that solution. Instead of (1) notifying NativeAnimatedTurboModule and (2) sending the onScroll event to JavaScript, we're only doing (1) (i.e: notifying NativeAnimatedTurboModule).
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D37257719
fbshipit-source-id: 7dea3cf8b9ae78f6b0fd40414b8f224d43367a5a
Summary: Some files relying on -include_pch and therefore they miss Foundation.h and UIKit.h includes. This diff is fixing missing imports
Reviewed By: rmaz
Differential Revision: D37140239
fbshipit-source-id: bc57921e0c8365e0e9a5a571d607ba40ff1b31f3
Summary:
Problem - Error when trying to publish to Apple Store in debug scheme
Error thread - https://github.com/facebook/react-native/issues/31507
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General][Removed] - The diffs renames the required variable which was causing conflicts in names with Apple core SDK's
Pull Request resolved: https://github.com/facebook/react-native/pull/33153
Reviewed By: lunaleaps
Differential Revision: D34392529
Pulled By: sshic
fbshipit-source-id: 78387999f94e0db71f5d3dafff51e58d7d0c1847
Summary:
Changelog:
[iOS][Fixed] - Expose the extraData dict attached to JavaScript errors to the native ExceptionManager on iOS, similar to Android
Attaching the `extraData` dict to JavaScript crash reports is something that was done for Android only in 2019 (D16133080 (https://github.com/facebook/react-native/commit/3a825c036065783aae6fb734028f986dbead80d7)), and somehow we never really got around to adding it in iOS. This diff finally adds the capability to iOS as well. `extraData` can be used to attach various bits of data to a crash report for better debugging and categorization. As with the Android implementation, `extraData` is not attached if the `reportException` API is not used.
Reviewed By: dmitryrykun
Differential Revision: D35743658
fbshipit-source-id: de4060cb6e514db1d85907441a8962f98e9b8392
Summary:
Changelog: [iOS][Internal] Add api to disable New validation reporting
Previously `RCTNewArchitectureValidationSetEnabled` was not set to false once it was set to true when a use is in app-wide Bridgeless mode.
This resulted it being in an incorrect state if a user:
1) Opens RN while in app-wide Bridgeless enabled
2) Logout
3) Re-login as another user without killing the app.
The fix is to set `RCTNewArchitectureValidationSetEnabled(RCTNotAllowedValidationDisabled)` in FBReactModule initialization.
Reviewed By: RSNara
Differential Revision: D35233335
fbshipit-source-id: 82a2c2ed030df5d68267a40b14322e652eb29e96
Summary:
Changelog: [iOS][Internal] Refactor: Migrate Logbox surface initialization to Fabric when available, in Bridge and Bridgeless modes
# Why
This diff main purpose is to add `RCTErrorNewArchitectureValidation(RCTNotAllowedInAppWideFabric)` in `RCTSurface`, to ensure Paper surfaces are never created in FBiOS.
# The Situation
Before this diff, in Bridged Fabric, `[RCTLogbox show]` initializes a Paper `RCTSurface`, [using `[RCTLogBoxView initWithWindow]`](https://github.com/facebook/react-native/blob/main/React/CoreModules/RCTLogBoxView.mm#L46))
In this diff, in Bridged and Bridgeless Fabric, `[RCTLogbox show]` initializes a Fabric `RCTFabricSurface`.
Before this diff, in Bridgeless Fabric, RCTLogBox posts a "CreateLogBoxSurface" notification to RCTInstance.
In this diff, the notification hack is replaced by the same `RCTFabricSurface` initialization above.
Behavior is the same.
Reviewed By: RSNara
Differential Revision: D35177311
fbshipit-source-id: 6de418af8a01f914c9a806bb8d74915015f9087a
Summary:
Changelog: [iOS][Internal] Remove RCTNotAllowedInBridgeless validation for RCTRegisterModule
In the TurboModule system, `RCTRegisterModule` gets called for all `RCTBridgeModules` that calls `RCT_EXPORT_MODULE()` and it works fine in Bridgeless mode.
Reviewed By: RSNara
Differential Revision: D35203039
fbshipit-source-id: 8ae2be4487fe21653a7f1628fa92606a7d36d467
Summary:
Changelog: [iOS][Internal] Rename RCTNotAllowedInFabric to RCTNotAllowedInAppWideFabric
Clarify that methods marked with `RCTNotAllowedInAppWideFabric` are only NOT available when Fabric is app-wide (which is necessary for app-wide Bridgeless mode). These methods may still be called in apps with legacy pre-Fabric surfaces.
Reviewed By: RSNara
Differential Revision: D35194789
fbshipit-source-id: e16fa54d22ea67be995e93f6ff60567a117398be
Summary:
Changelog: [Internal][iOS] Add validation reporting APIs for unexpected uses of Paper when Fabric is enabled
## RCTNotAllowedInBridgeless
Previously, we only had violation reporting APIs for when **Bridge APIs** are used in **Bridgeless mode**, which was only enabled in Bridgeless mode.
## RCTNotAllowedInFabric
This diff adds violation reporting APIs to use when **pre-Fabric Bridge APIs** are used in **Bridge or Bridgeless mode**. This allows us to add RCTAssert/RCTError/RCTLog to more APIs in Bridge mode. The main purpose is to distinguish between Bridge APIs that still work in Fabric, versus Bridge APIs that are no longer used in Fabric, so that the latter can be removed.
Reviewed By: philIip
Differential Revision: D35015758
fbshipit-source-id: 35366bc5143a59ee9a16d75da4de546ebfe250e6
Summary:
When starting the surface, _propagateStageChange is called. This checks the delegate to call surface:didChangeStage: on it.
When initWithSurface:sizeMeasureMode: is called after start, then the delegate will be nil and thus not be called.
This turns it around so a delegate is present for the surface to propagate its state to.
This fixes RCTContentDidAppearNotification not getting posted otherwise.
## Changelog
[iOS] [Fixed] - Post RCTContentDidAppearNotification with new arch
Pull Request resolved: https://github.com/facebook/react-native/pull/33402
Test Plan:
I found it best to set a breakpoint in XCode to where RCTContentDidAppearNotification is being posted.
Prior to the patch that breakpoint will not be called. After applying the patch, it will be called.
Reviewed By: philIip
Differential Revision: D34753329
Pulled By: ShikaSD
fbshipit-source-id: cc44a4c3a787d49e22e9d0c3a82c0f11ed281a0a
Summary:
Changelog: [iOS] Add `_viewRegistry_DEPRECATED addUIBlock` to replace `uiManager addUIBlock` for Fabric migration
Allow people to directly replace `uiManager addUIBlock` with `_viewRegistry_DEPRECATED addUIBlock` when they migrate the method in RCTViewManagers.
Currently we add an if check in the RCTViewManager, which makes migration a bit harder for OSS.
```
if (self.bridge) {
[self.bridge.uiManager addUIBlock:^(__unused RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry) {
work();
}];
} else {
RCTExecuteOnMainQueue(work);
}
```
Reviewed By: sammy-SC
Differential Revision: D34532609
fbshipit-source-id: 19647fea03be8fd71d8f46dfe216275894d8165c
Summary:
Changelog: 2/n Refactor RCTModuleData to use RCTBridgeModuleDecorator, to attach synthesize ivars to non-TurboModule RCTBridgeModules in Bridge mode
I decided to not include the `RCT_PROFILE_BEGIN_EVENT`s in [RCTBridgeModuleDecorator attachInteropAPIsToModule] because people don't override the set methods for the 4 synthesize ivars, so setting each ivar should be instant.
Reviewed By: RSNara
Differential Revision: D34438180
fbshipit-source-id: 871ac4cadd7a36821dac1274f0645c19d63943fc
Summary:
Changelog: [iOS][Internal] Refactor CxxBridge: Introduce RCTBridgeModuleDecorator to attach synthesize ivars to RCTTurboModules, in Bridge mode
This doesn't change any logic. RCTBridgeModuleDecorator was created to consolidate several nearly identical implementations of the `attachInteropAPIsToModule` method to one place.
Most importantly, it allows us to attach interop APIs in RCTBridgeModuleDecorator to RCTViewManagers in diff 4/4, using `attachInteropAPIsToModule`. Before this stack, these four synthesize ivars in RCTViewManagers are nil in Bridgeless mode, and point to instances in Bridge mode.
# Context
These are used in RCTBridgeModules to access APIs for view managers. These APIs are necessary and compatible with Bridgeless mode.
* synthesize viewRegistry_DEPRECATED
* synthesize bundleManager
* synthesize callableJSModules
* synthesize moduleRegistry
Reviewed By: RSNara
Differential Revision: D34437802
fbshipit-source-id: b773d511cf877d4896436fabf4893c978e5f8dd9
Summary:
Changelog: [Internal]
In the new architecture, when an interop component is being called, log instead of warn/error, since at the moment we expect this to happen often.
Reviewed By: fkgozali
Differential Revision: D34252666
fbshipit-source-id: 971156a1cd9ef9b788f677c49fa2c55bd86ad4fa
Distiller
Distiller
Distiller
Distiller
Rob Hogan
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Lorenzo Sciandra
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Distiller
Paige Sun
lbaldy
Pieter De Baets
Ramanpreet Nara
Ruslan Latypov
Arinjay Sharma
Gijs Weterings
Vincent Riemer
Danilo Bürger
Samuel Susla
Amy Lee