Summary:
Changelog: [internal]
"onDismiss" event wasn't called in Fabric. This diff adds it.
Paper implementation of Modal uses `RCTEventEmitter` instead of callback to deliver the event. To align better with Paper, Fabric will follow this pattern.
Reviewed By: shergin
Differential Revision: D26911312
fbshipit-source-id: b0de619c5a02c3378d1f7ac3ce1b705bb5fb634d
Summary:
We need to register a root view component here synchronously because right after we start a surface, it can initiate an update that can query the root component.
This fixes an issue in Fabric Surface caused by a data race between registration of root component and concurrent call to `constraintLayout` on the main thread.
This how it happens:
1. We call `startSurface` on some background thread;
2. As a result `[RCTFabricSurface start]` schedules a lambda to the main thread that will register a root component.
3. Meanwhile some other code on the main thread changes the layout and requests the relayout of a surface.
4. Because the surface is already running, the `constraintLayout` method works as expected and schedules a mutation instruction that changes the size of the root component.
5. The mutation instruction is being delivered synchronously on the main thread and fails because the root component was not yet registered. Boom.
For now, it's not fully clear how to solve this problem properly. I suspect that the good solution would be to shift the responsibility of registering a root component to the MountingCoordinator side (so, we will have an explicit instruction prescribing creating and registering the root component). This will probably be a quite invasive change though.
For now, I think it's fine to make the block synchronous to avoid the race.
Reviewed By: mdvacca
Differential Revision: D26802418
fbshipit-source-id: d49484c90d1ac61ac595caf486562fc6f4843e2f
Summary:
We want to be able to instrument touch processing delays in JS, which does not have access to systemUptime; therefore we want a UNIX timestamp, which JS has access to and can compare to the touch time.
It only matters that there is relative consistency between multiple touch events in JS, which is still the case; so this should have no impact on product code.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26705430
fbshipit-source-id: 2acd52ae5873a44edf1e0cb126a9a6c87203d8fa
Summary:
changelog: [internal]
### When does leak happen?
Leak happens anytime a callback isn't executed inside native module, it will never get cleaned up.
Imagine a native module with method that takes onSuccess and onFail callbacks. Only one of them will be called at any time and the other one will leak.
### Why does it leak?
It leaks because when `CallbackWrapper` is created using `CallbackWrapper::createWeak`. Inside `CallbackWrapper::createWeak`, the newly created object is inserted into `LongLivedObjectCollection`. This object collection will keep it alive until `CallbackWrapper::destroy` is called, which isn't called in case closure isn't executed.
### Solution
Introduce class RCTBlockGuard which ties cleanup of resources to lifetime of the block.
Reviewed By: RSNara
Differential Revision: D26664173
fbshipit-source-id: 9348f7c39eb317cf1e8e5d59e77a378e5e04f3eb
Summary:
(This is the second version of the diff that includes additional checkes and syncronization around `start` and `stop` methods).
This diff migrates iOS renderer to using SurfaceHandler directly instead of calling Scheduler methods.
Major changes:
* RCTFabricSurface only stores a SurfaceHandler and platform-specific parts (such as a UIView instance and TouchHandler); all other pieces of state are stored in SurfaceHandler object.
* All the state changes initiated from RCTFabricSurface are performed via calling corresponding methods on SurfaceHandler (bypassing RCTSurfacePresenter, RCTScheduler, Scheduler, and UIManager).
* Every RCTSurfaceHandler is responsible for observing `UIContentSizeCategoryDidChangeNotification` and communicating the change down to SurfaceHandler. (Besides code simplifications it's more correct because on iOS the DPI actually depends on a particular UIScreen/UIWindow associated with UIView and *not* global for the whole app.)
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: ShikaSD
Differential Revision: D26614577
fbshipit-source-id: 428f4c4cc1ca46cd97be0413e9279e29a54807f6
Summary:
Since iOS 14 refresh control is sometimes visible when it shouldn't. It seems to happen when it is removed and added back to the window. This repros easily when using react-native-screens with react-navigation tabs. Inactive tabs are detached from the window to save resources.
Calling endRefreshing when refresh control is added to the window fixes the layout. It will also be called on first mount where it is not necessary, but should be a no-op and didn't cause any issues. I also decided to call it for all ios versions, although it is only needed on iOS 14+ to avoid forking behavior more.
## Changelog
[iOS] [Fixed] - Fix RefreshControl layout when removed from window
Pull Request resolved: https://github.com/facebook/react-native/pull/31024
Test Plan:
Before:
https://user-images.githubusercontent.com/2677334/108666197-93ea5a80-74a4-11eb-839b-8a4916967bf8.mov
After:
https://user-images.githubusercontent.com/2677334/108666223-9ea4ef80-74a4-11eb-8489-4e5d257299c8.mov
Reviewed By: shergin
Differential Revision: D26590759
Pulled By: PeteTheHeat
fbshipit-source-id: b8c06068a24446b261cbeb88ff166289724031f1
Summary:
Changelog: [internal]
Prevent crash when casting of state to `ImageShadowNode::ConcreteState` fails. This doesn't fix root cause of the problem but stops the app from crashing.
Reviewed By: JoshuaGross
Differential Revision: D26604807
fbshipit-source-id: 17a2ead56ac68e560070ed4defd364a9d1dfd1e8
Summary:
Changelog: [Internal]
This moves enabling/disabling fast refresh off of `bridge.enqueueJSCall` in bridgeless mode.
Reviewed By: sammy-SC
Differential Revision: D26290378
fbshipit-source-id: ed8a3389b9812cedf7181971656dacd98ff7ecfd
Summary:
Changelog: [Internal]
This diff allows `RCTDevSettings` to access bundleURL directly instead of relying on the bridge.
Reviewed By: RSNara
Differential Revision: D26289592
fbshipit-source-id: e22e583e51323bfe66f6e424d9f47edf71b1ed9f
Summary:
Changelog: [Internal]
Problem:
The Bridge holds and exposes the bundle URL for native modules to inspect via `bridge.bundleURL`.
Solution:
This follows the same pattern we've been using for attaching objects to native modules: declare a protocol with the required object, attach that object during module init.
Differential Revision: D26289581
fbshipit-source-id: 35ae83912e392be84f55c8d056c8da15ba75060f
Summary:
This is a backout of D24290778 (https://github.com/facebook/react-native/commit/33d6091cac5ed2f8cef06d4740c8b42fcd107271).
The original diff causes T85097896 and probably T85069775. For now I am going to unland this, and then I will re-land this after the cut with a fix.
Original commit changeset: 62b600c3f1b2
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26515027
fbshipit-source-id: 057663b092e6b1d74db83c40d656b2ae142bd622
Summary:
Previous implementation was now correct because it assumed that all the constant values are aligned between RN and UIKit.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D26476566
fbshipit-source-id: 07bbe14d887d446bae34f448f5e62c3075fb0979
Summary:
This diff migrates iOS renderer to using SurfaceHandler directly instead of calling Scheduler methods.
Major changes:
* RCTFabricSurface only stores a SurfaceHandler and platform-specific parts (such as a UIView instance and TouchHandler); all other pieces of state are stored in SurfaceHandler object.
* All the state changes initiated from RCTFabricSurface are performed via calling corresponding methods on SurfaceHandler (bypassing RCTSurfacePresenter, RCTScheduler, Scheduler, and UIManager).
* Every RCTSurfaceHandler is responsible for observing `UIContentSizeCategoryDidChangeNotification` and communicating the change down to SurfaceHandler. (Besides code simplifications it's more correct because on iOS the DPI actually depends on a particular UIScreen/UIWindow associated with UIView and *not* global for the whole app.)
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24290778
fbshipit-source-id: 62b600c3f1b2e66a7513481404af941ef8b78bec
Summary:
Changelog: [internal]
To avoid typecasting, let's return `UIViewContentMode`. This way we get rid of a condition to check if contentMode is repeat.
Reviewed By: JoshuaGross
Differential Revision: D26252431
fbshipit-source-id: 94ef7af1a76f13c91b696d57ceecc2453bbc9d8d
Summary:
When the TurboModule system is enabled, all TM-compatible NativeModules should not go through the legacy system. We have this filtering elsewhere in the bridge, but not for eagerly initialized NativeModules with custom initializers (i.e: modules returned from extraModulesForBridge). This diff adds that filtering for modules returned from extraModulesForBridge.
NOTE: NativeModule initializers can still perform side-effects. So, it's still best to not create the TM-compatible NativeModule inside extraModulesForBridge. This diff just adds an additional layer of defense within the bridge.
Changelog: [Internal]
Differential Revision: D26264531
fbshipit-source-id: 26179d9094f67d6d6315a65cdb2dc614e5397bf3
Summary:
This is a Fabric-compliant implementation of `JSResponder` feature. To make it work e2e we also need to update FabricRenderer in React repository. But before we can do this, we need to ship the native changes.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D24630027
fbshipit-source-id: 70c30e1250b554d83862956b536714704093072f
Summary:
Changelog: [Internal]
This removes a couple dependencies on bridge in dev menu native module. So far reloading works, will get inspector and other options working in follow ups.
Reviewed By: sammy-SC
Differential Revision: D26240391
fbshipit-source-id: 7c9d585f4efa6cc9db995ef8a33865831bc8d526
Summary:
Live reload was removed as a user facing feature in 2019 in favour of fast refresh. The native code was left in case "automation" relied on it. I'm quite sure no automation is using this dev feature. Remember, the dev feature made it so that hitting save in a text editor auto reloaded in metro.
Cleanup the native implementation.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26239628
fbshipit-source-id: 7f61c7204727bb2d739600a459f69c72842265c5
Summary:
IIUC `isDebuggingRemotely` will never be set when `isDeviceDebuggingAvailable` is true. This was just a failsafe so that noone was trapped in an intermediate state.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26239570
fbshipit-source-id: 8db6b022a0ea581216a1fa97b9d41f5ab6160562
Summary:
Changelog: [internal]
Provide a default plugin function that provides core `RCTModalHostViewComponentView`. This makes for easier setup for standalone app migration.
Reviewed By: JoshuaGross, shergin
Differential Revision: D26150091
fbshipit-source-id: d39723b99c590d1a33fb7b628f809aa12b2f7589
Summary:
In bridegless mode, `setBridge` is never called on native modules. This diff refactors out some setup logic into the module's init method.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D26211620
fbshipit-source-id: 7a9910198852dea5ac679762aa93dfea6fc47d70
Summary:
This addesses a few issues I noticed while migrating my app to the new build-time codegen on iOS.
1. I noticed random failures because of codegen on iOS. This is mostly due to the fact the codegen output files are not specified in the xcode script. The only reason it works relatively fine currently is because the codegen output is inside the input files directory. This has the side effect of causing files to be regenerated every build, then causes all core modules to be recompiled which adds up a significant amount of time to rebuilds. To fix this I added the generated files to the script phase output and moved the FBReactNativeSpec dir outside of the codegen source (Libraries). I moved it to the React directory as this seemed to make sense and is where a lot of iOS files are as well as the core modules. Note this might require internal changes. This removes the circular dependency between our build phase input and output so consecutive builds can be cached properly.
2. Add `set -o pipefail` to the xcode script, this helped propagate errors properly to xcode because of the `| tee` pipe so it fails at the script phase and not later with a header not found error. Also add `2>&1` to pipe stderr to stdout so errors are also captured in the log file.
3. Add the `-l` flag to the bash invocation to help finding the yarn binary. With my setup yarn is added to the system PATH in my user .profile. Adding this file will cause bash to source the user environment which xcode scripts does not by default. I think this will help with most setups.
4. If yarn is not found the `command -v yarn` would make the script exit without any output because of the -e flag. I made a change to ignore the return code and check later if YARN_BINARY is set and have an explicit error message if not.
## Changelog
[iOS] [Fixed] - Make codegen more reliable on iOS
Pull Request resolved: https://github.com/facebook/react-native/pull/30792
Test Plan:
Tested various project states to make sure the build always succeeds in RN tester:
- Simulate fresh clone, remove all ignored files, install pods, build
- Build, delete FBReactNativeSpec generated files, build again
- Build, build again, make sure FBReactNativeSpec is cached and not rebuilt
- Make the script fail and check that xcode shows the script error logs properly
Note: Did not test fabric
Reviewed By: fkgozali
Differential Revision: D26104213
Pulled By: hramos
fbshipit-source-id: e18d9a0b9ada7c0c2e608d29ffe88087f04605b4
Summary:
Changes:
* RCTMountingManager got two dedicated methods that designate some external view as a Surface viewport;
* The wiring between views now happens right in `start` and `stop` methods (because there is no reason to do it later); now it's symmetrical and straightforward;
* A couple of Surface legacy statuses were removed; they are not used and do not convey useful information anyway.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26095434
fbshipit-source-id: 37828e093a40a1ad22ad78f110c18a8f27781ba7
Summary: Changelog: [iOS] Only show Dev Menu on shake if RN view is visible
Reviewed By: fkgozali
Differential Revision: D26138659
fbshipit-source-id: e2db287728675c7ead5fcbf569ed591638e2187e
Summary:
Before this change, `mountingOverrideDelegate` was proxied via `Scheduler::startSurface` and `ShadowTree::ShadowTree` constructor down to `MountingCordinator`. Now we set it on the `MountingCoordinator` directly.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26049076
fbshipit-source-id: 7f1ecf2c8b6f264a7e59d19881464fe529c53d30
Summary:
In RN, a <Text> node can be marked "not-accessible" by specifying `accessible=false`. This diff implements this for Fabric.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24386225
fbshipit-source-id: 576f4cbe1916e90034f8c9f06f19cea1992e7abd
Summary:
Normally, UIScrollView adjust `contentOffset` inside `setContentSize` to prevent over-scroll. In most cases, it works fine but in a case when the ScrollView is in the middle of user interaction the over-scroll is perfectly acceptable. So, to work around this issue we disable `setContentOffset` when we update `contentSize` and the gesture interaction is in progress.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26001133
fbshipit-source-id: 19b67289b2c142facd4f516a740180c87f90bc2d
Summary:
This is the iOS native implementation of sendAccessibilityEvent for Fabric.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D25857401
fbshipit-source-id: e57d2e7fd45052bcf05cee7d0cb6c55ee974f358
Summary:
This pull request is to fix https://github.com/facebook/react-native/issues/30258.
After an investigation, I found out the scroll offset of the list seems to be calculated incorrectly due to a workaround in the source code.
Instead of fixing the `calculateOffsetForContentSize`, I chose to remove it, the reason why I do so is because this workaround is for fixing the offset when `contentSize` is set manually, but according to the source code, there is no interface for a react-native user to set the `contentSize` of ScrollView, it is just set to `GCSizeZero` and will never be changed ([ref](https://github.com/facebook/react-native/pull/30647/files#diff-cf6f991f585ebf4cfdd555fe474e1f9ce40c2e4f823fc3f42b549414639c8c30L304)).
Also I changed the function name from `updateContentOffsetIfNeeded` to `updateContentSizeIfNeeded` according what the function is actually doing.
## 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] - Incorrect ScrollView offset on update
Pull Request resolved: https://github.com/facebook/react-native/pull/30647
Test Plan:
1. Create a fresh new project named `testapp` using npx create-react-native-app
2. Apply example code to `testapp` from https://snack.expo.io/lokomass/flatlist
3. Run `testapp` on iOS emulator and reproduce the bug
4. Make changes to files in `testapp/node_modules/react-native/`
5. Rebuild `testapp` and run on iOS emulator again, the bug is no more exist
6. Apply changes from step 4 to react-native, make a pull request.
#### Screenshots
Before: The scroll offset is incorrect after children of FlatList has changed
https://user-images.githubusercontent.com/48589760/103155130-b54a0580-47d7-11eb-97af-bdfd3e728714.mov
After: No more incorrect scroll offset if children of FlatList has changed
https://user-images.githubusercontent.com/48589760/103155091-6ef4a680-47d7-11eb-89fa-6f708bfef1c9.mov
Reviewed By: sammy-SC
Differential Revision: D25732958
Pulled By: shergin
fbshipit-source-id: dac6eff15ac3bbfec502452ac14b3d49fee76c29
Summary:
Changelog: [internal]
Previous implementation, (D25684453 (https://github.com/facebook/react-native/commit/eba31d285847ef70cf22ce83fe013efa95416e3c)) which was backed out, had a bug that caused assert to fail. It was restarting internal state `RCTSurfaceTouchHandler` and lead to inconsistency.
This is because touch event is a state machine and before "moved" is called, touch event had to "began". With internal state being restarted, that wasn't the case.
In the previous implementation the internal state was incorrectly restarted because I thought touches would not longer receive touch events after declaring that UIGestureRecogniser shouldn't be recognised simultaneously.
In this diff I take a different approach. Instead of restarting the internal state, the recogniser is first disabled and then enabled again. This cancels any ongoing touches instead of manual cleanup.
### Other approaches
In the UIViewController, we could listen to `viewWillDisappear` message and cancel any ongoing touches on the surface. This would work as well (it would prevent Pressable components from being highlighted during interactive view controller dismissal) however we would need to expose existence of `RCTTouchHandler` to the view controller. This approach is also discouraged in D15734129 (https://github.com/facebook/react-native/commit/36307d87e1974aff1abac598da2fd11c4e8e23c1).
Reviewed By: shergin
Differential Revision: D25685536
fbshipit-source-id: ad226c82e73f841c1ebd9b92738847cc5cdc21d1
Summary:
## Description
Suppose this was the codegen declaration before:
```
rn_library(
name = "FooModule",
native_module_spec_name = "FBReactNativeSpec",
codegen_modules = True,
# ...
)
```
Previously, this would generate the following BUCK targets:
- generated_objcpp_modules-FooModuleApple
- generated_java_modules-FooModuleAndroid
- generated_java_modules-FooModule-jniAndroid
## Changes
We will now generate:
- FBReactNativeSpecApple
- FBReactNativeSpecAndroid
- FBReactNativeSpec-jniAndroid
This matches the naming scheme of the old codegen.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D25680224
fbshipit-source-id: 617ac18fd915f3277f6bd98072d147f20fb193e5
Summary:
This replaces the internal core implementation of `setState` with the new `updateStateWithAutorepeat` which is now the only option.
In short, `updateStateWithAutorepeat` works as `setState` with the following features:
* The state update might be performed several times until it succeeds.
* The callback is being called on every retry with actual previous data provided (can be different on every call).
* In case of a static value is provided (simple case, not lambda, the only case on Android for now), the same *new*/provided value will be used for all state updates. In this case, the state update cannot fail.
* If a callback is provided, the update operation can be canceled via returning `nullptr` from the callback.
This diff removes all mentions of the previous state update approach from the core; some other leftovers will be removed separatly.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25695600
fbshipit-source-id: 14b3d4bad7ee69e024a9b0b9fc018f7d58bf060c
Summary:
Original commit changeset: a3324b5df6e6
ConcreteStateTeller is being replaced with a new built-in state autorepeat mechanism.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25687697
fbshipit-source-id: 849bb8b6c244b17a49b5fe140346d90a9a7e52ff
Summary:
Original commit changeset: 4c0d7b249175
ConcreteStateTeller is being replaced with a new built-in state autorepeat mechanism.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25687698
fbshipit-source-id: b7eac808db1acdc7ef3297313ac352162bdccd5b
Summary:
Original commit changeset: b82301e5db48
ConcreteStateTeller is being replaced with a new built-in state autorepeat mechanism.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25687694
fbshipit-source-id: 17a737d394cb7f460b239be421466981bc46f995
Summary:
Original commit changeset: 2b82e2d43cc6
ConcreteStateTeller is being replaced with a new built-in state autorepeat mechanism.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25687699
fbshipit-source-id: d318155509b00064e29fc32efe893151a9723fd5
David Biedenbach
Samuel Susla
Valentin Shergin
Joshua Gross
Peter Argany
Janic Duplessis
Ramanpreet Nara
Alan Kenyon
Andres Suarez
Mike
