Summary:
Seems a ScrollView sometimes calls the delegate in own destructor; and seems that in some configurations the delegate is also already destroyed at this point. I am not sure if this a bug in UIKit or not, but seems the fix is easy, we just have to clear the ScrollView's delegate on the delegate's deallocation.
This issue is also looks similar:
https://stackoverflow.com/questions/18778691/crash-on-exc-breakpoint-scroll-view/19011871
Changelog: [iOS] [Fixed] - Fixed crash in RCTScrollViewComponentView
Reviewed By: sammy-SC
Differential Revision: D17924429
fbshipit-source-id: 5727bca9f028e28f76f60304c187ee39eb6e1856
Summary:
For components to be used with LegacyViewManagerInterop they need to be added to a white list.
This makes it possible to test it out and assure proper functionality.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D17905453
fbshipit-source-id: 4e8e53a1898b38b2c9f01e7fc9e3527bd7004ffb
Summary:
For components to be used with LegacyViewManagerInterop they need to be added to a white list.
This makes it possible to test it out and assure proper functionality.
Reviewed By: shergin
Differential Revision: D17905413
fbshipit-source-id: f5ab523cca6227e99a7607ca1927005392b1ae36
Summary:
The component RCTRefreshControl was renamed to PullToRefreshView (for Paper). Now only old Objective-C class names have the old name, which is okay.
Changelog: [Internal] [Changed] - The internal name of PullToRefresh component was changed from `RCTRefreshControl` to `PullToRefreshView` (No public API changes)
Reviewed By: rickhanlonii
Differential Revision: D17456225
fbshipit-source-id: a8db99ddd507377d8c98b26707a3b9fae483d20c
Summary:
The Android makefiles had hard-coded paths to hermes-engine, sometimes looking in `node_modules` and other times looking in `..`. This commit implements the Node module resolution algorithm (see common.mk), which handles both of these cases and also looks further up the root if necessary, handling the case when the `hermes-engine` npm package is hoisted.
This commit does three things:
- Defines `find-node-module` and uses it in the makefiles to find `hermes-engine`
- Removes the unused `/path/to/hermes-engine/include` paths since this directory does not exist and should be `/path/to/hermes-engine/android/include` (`android`)
- Moves the definition of `REACT_NATIVE` in the makefiles to the top. It was defined after every `$(CLEAR_VARS)` invocation but was not actually cleared anyway. `$(CLEAR_VARS)` resets only `LOCAL_*` variables in this list: https://android.googlesource.com/platform/build/+/7dc45a8/core/clear_vars.mk
## Changelog
[Internal] [Changed] - Android Makefiles look for hermes-engine using Node's module resolution algorithm
Pull Request resolved: https://github.com/facebook/react-native/pull/26820
Test Plan: Run `./gradlew :ReactAndroid:installArchives`, which requires the hermes-engine paths to be correct in order to find the headers.
Differential Revision: D17923671
Pulled By: cpojer
fbshipit-source-id: 9238b8718a94080db1abbba6375a6a1d484c871d
Summary:
**Context**
For method calls from JS to Objective C, we have to convert JS values to ObjC objects/primitives. Before we can call our ObjC methods, we need to run both the ObjC primitives and objects through `RCTConvert`. This is necessary, because we sometimes convert `NSDictionary`s to special Objective C objects. Apparently, in `RCTTiming`, we also do the same with `double` (i.e: we convert a `double` arg to another `double` type with different meaning).
**Problem**
`RCTTiming` used `RCTConvert` to convert `double`s into `NSTimeInterval` (also a double). The conversion is defined like this:
```
// i.e: division by 1000
RCT_CUSTOM_CONVERTER(NSTimeInterval, NSTimeInterval, [self double:json] / 1000.0)
```
This diff implements the support necessary to make this work. For completeness, I also implemented the same functionality for `BOOL`s.
Changelog: [iOS][Fixed] Improve method argument RCTConvert logic
Reviewed By: mdvacca
Differential Revision: D17887915
fbshipit-source-id: 3246fdbf4db7e96911f16460d92448b1f1e99444
Summary:
We are going to need to change some of these APIs to use refs instead of findNodeHandle. I figured I'd start by adding some tests
Changelog:
[Internal] Adding tests for TextInput
Reviewed By: yungsters
Differential Revision: D17892806
fbshipit-source-id: f59ff99fa4d064239f171acb64a8441e07bb71c1
Summary:
In the legacy system, when NativeModules are supposed to be initialized on the main queue, we do the following synchronously on the main thread:
1. Done: Attach bridge on main queue
2. Register the NativeModule for frame updates on main queue
3. Post Notification that NativeModule was created on main queue
4. Attach methodQueue on main queue
5. Call new on main queue
`[RCTModuleData instance]` is the entrypoint for all of this logic.
We probably shouldn't synchronously execute all this initialization on the main queue, because it can lead to deadlocks down the road. Therefore, this diff makes it so that we still call `new` on the same thread. However, we do all other initialization in the main thread, if that's required.
Changelog: [iOS][Fixed] TurboModule initialization on the main queue
Reviewed By: PeteTheHeat
Differential Revision: D17867583
fbshipit-source-id: a88412ee1e3d93a4f9b5ab0b4dd8fc5213fa91f8
Summary:
These were being cast to a NativeComponent but that is no longer accurate. `requireNativeComponent` returns the type of `HostComponent` now which is more accurate. We don't need the cast through `any` anymore.
In order to know that I found all the callsites, I ran this command to find these:
```
grep -r "requireNativeComponent" react-native-github -C 5 | grep 'any'
```
Changelog:
[Internal]
Reviewed By: cpojer
Differential Revision: D17864165
fbshipit-source-id: 3774d6d47d7bb0d885cc1a1352f81fec7d3bca0d
Summary:
Replaces the use of `framesToPop` in `_allocateCallback` (in `__DEV__` only) with statically accessing the second frame of the stack.
Changelog: [Internal]
Reviewed By: cpojer
Differential Revision: D17877261
fbshipit-source-id: 8e4d0eb2ed7984b66a99752fb21f7909474fda8f
Summary:
Currently the react native framework doesn't handle the accessibility state changes of the focused item that happen not upon double tapping. Screen reader doesn't get notified when the state of the focused item changes in the background.
To fix this problem, post a layout change notification for every state changes on iOS.
On Android, send a click event whenever state "checked", "selected" or "disabled" changes. In the case that such states changes upon user's clicking, the duplicated click event will be skipped by Talkback.
## Changelog:
[General][Fixed] - Announce accessibility state changes happening in the background
Pull Request resolved: https://github.com/facebook/react-native/pull/26624
Test Plan: Add a nested checkbox example which state changes after a delay in the AccessibilityExample.
Differential Revision: D17903205
Pulled By: cpojer
fbshipit-source-id: 9245ee0b79936cf11b408b52d45c59ba3415b9f9
Summary:
This pull request replaces the last remaining Unix headers in `JSBigString` with their equivalent Folly Portability headers, and replaces the calls to `getpagesize()` with `sysconf(_SC_PAGESIZE)` since Folly Portability is missing that function.
The work to get this building on windows was mostly done by acoates-ms, this pull request just adds the finishing touches.
## Changelog:
[General] [Fixed] - Fixed `JSBigString` not compiling on Windows due to Unix-specific headers
Pull Request resolved: https://github.com/facebook/react-native/pull/26826
Test Plan: Compiled with Clang and with MSVC (2017)
Differential Revision: D17903214
Pulled By: cpojer
fbshipit-source-id: 230f8fb410fa81d8f13d8b6ccf1147cfc70358bf
Summary:
- Using "System.getenv" allows to specify any entry file using environment variables and without modifying gradle file. Example:
export ENTRY_FILE="another_entry_file.js"
./gradlew assembleDebug
- This functionality is also more align with iOS implementation that uses 'if [[ "$ENTRY_FILE" ]]; then'. See https://github.com/facebook/react-native/pull/23667 for more details.
- Possibility to define entry file on CI without modifying sources (Example: project like [pixels-catcher](https://www.npmjs.com/package/pixels-catcher) requires different entry file)
## Changelog:
[Android] [Added] - Custom entry file on android using `ENTRY_FILE` environment variable
Pull Request resolved: https://github.com/facebook/react-native/pull/26769
Test Plan:
- Create a project from template
- Define `ENTRY_FILE` environment variable
```
export ENTRY_FILE="anotherIndexFile.js"
```
- Build android
```
./gradlew assembleDebug
```
Expected result: App contains bundle file that starts from `anotherIndexFile.js` file.
Differential Revision: D17903165
Pulled By: cpojer
fbshipit-source-id: 6b7cdf229918d101c170aa5fbdca6f3ef293d41c
Summary:
The Gradle build file looks up jsc-android and hermes-engine using hard-coded paths. Rather than assuming the location of these packages, which are distributed and installed as npm packages, this commit makes the Gradle file use Node's standard module resolution algorithm. It looks up the file hierarchy until it finds a matching npm package or reaches the root directory.
## Changelog:
[Android] [Changed] - ReactAndroid's Gradle file uses Node's module resolution algorithm to find JSC & Hermes
Pull Request resolved: https://github.com/facebook/react-native/pull/26773
Test Plan: Ensure that CI tests pass, and that `./gradlew :ReactAndroid:installArchives` works. Printed out the paths that the Gradle script found for jsc-android and hermes-engine (both were `<my stuff>/react-native/node_modules/jsc-android|hermes-engine`).
Differential Revision: D17903179
Pulled By: cpojer
fbshipit-source-id: 9ac3ba509974f39f87b511d5bc3398451c12393f
Summary:
Deletes `getObjectValues` because there are no more references to it in `react-native`.
Changelog:
[Internal]
Reviewed By: TheSavior
Differential Revision: D17884311
fbshipit-source-id: c97caeed00fe94a6cc099fba7039d9defee719dd
Summary:
Deletes `isEmpty` because there are no more references to it in `react-native`.
Changelog:
[Internal]
Reviewed By: zackargyle, fred2028
Differential Revision: D17884310
fbshipit-source-id: 0554aee4044452b6c04f638f1ad762025eecd929
Summary:
Deletes `guid` because there are no more references to it in `react-native`.
Changelog:
[Internal]
Reviewed By: zackargyle
Differential Revision: D17882369
fbshipit-source-id: c3ee6d23e5fa233a7f5d2e2c7baef005384ea5b1
Summary:
Deletes `toIterator` because there are no more call sites.
Changelog:
[Internal]
Reviewed By: TheSavior
Differential Revision: D17879834
fbshipit-source-id: 95679d7504c044d0e842bfcbdc07a8c33268f5d6
Summary:
Deletes `mixInEventEmitter` and its dependencies that are no longer being used by anything in `react-native`.
Changelog:
[Internal]
Reviewed By: TheSavior
Differential Revision: D17879835
fbshipit-source-id: 45f30d21cb01365fcfc723cf564ebb47794ea176
Summary:
Deletes the `selectionState` prop from `TextInput`.
It does not provide meaningful value over `onBlur`, `onFocus`, and `selectionState`.
Changelog:
[Breaking][TextInput] Removing `selectionState` prop, use `onBlur`, `onFocus`, and `onUpdate` instead.
Reviewed By: zackargyle, TheSavior
Differential Revision: D17879667
fbshipit-source-id: 03a4e239406932adad898d6d2a092e3bc2e6b064
Summary:
In iOS 13, Apple made a change that results in video URLs returned by UIImagePickerController becoming invalidated as soon as the info object from the delegate callback is released. This commit works around this issue by retaining these info objects by default and giving the application a way to release them once it is done processing the video.
See also https://stackoverflow.com/questions/57798968/didfinishpickingmediawithinfo-returns-different-url-in-ios-13
Reviewed By: olegbl, mmmulani
Differential Revision: D17845889
fbshipit-source-id: 12d0e496508dafa2581ef12730f7537ef98c60e2
Summary:
Each React Native release has almost 1000 commits and we need to help make it easier for our releases by making sure we put changelogs in our diffs.
This will make changelogs required at land time for diffs touching files in react-native-github.
Reviewed By: zackargyle
Differential Revision: D17585190
fbshipit-source-id: a8ba65e6cc90bed47eeca3d58f16a1dc818c64aa
Summary:
In a previous diff I converted this line from a shared_ptr to a raw pointer. I'm documenting the reason why in code here to proactively document assumptions.
Changelog:
[[Internal]]
Reviewed By: shergin
Differential Revision: D17884942
fbshipit-source-id: 25e29a5e3ff0695d081c8d46c1ceaa6458248e26
Summary: These types are more accurate
Reviewed By: lunaleaps
Differential Revision: D17862010
fbshipit-source-id: 84dfcade35c21b7be49db46ae021819dda020c98
Summary: These types were wrong, this is a HostComponent, not a ReactNative.NativeComponent
Reviewed By: lunaleaps
Differential Revision: D17862305
fbshipit-source-id: e1e7acc7a5892f124b07cdc39d73d6ce7d414063
Summary: Tear down FabricUIManager more aggressively. The rest of the ivars here we can't set to null (yet) because they're marked as final.
Reviewed By: mdvacca
Differential Revision: D17878342
fbshipit-source-id: a1c8ace2603750ac39c3d97e1aca6c486ba5fb79
Summary:
These components had props that were poorly typed and let many things through. This diff tightens that all up.
The main difference in this diff is using `{...BaseProps, ...LocalProps }` instead of `BaseProps & LocalProps`.
The majority of the changes in this diff is reducing duplicated prop definitions. For example, FlatList defines a bunch of props that VirtualizedList also defines. Since FlatList extends those props, using spread now means that FlatList can't duplicate those props. So I've moved the definitions to the correct base file and deleted the duplicates.
Changelog:
[Internal]
Reviewed By: lunaleaps
Differential Revision: D17824459
fbshipit-source-id: 089ac4c58c3c9f70a9f28e517f2e9ecd8aab1a50
Summary:
Data doesn't have to be an array. data and getItemCount can take any arbitrarty data, as long as they expect the same thing. This should probably be parameterized with a generic but that is an improvement for another day.
It is worth noting the comment explanation for the defintion of `data` and `getItem` in VirtualizedList: https://fburl.com/u7ldzaa8
```
/**
* The default accessor functions assume this is an Array<{key: string} | {id: string}> but you can override
* getItem, getItemCount, and keyExtractor to handle any type of index-based data.
*/
data?: any,
/**
* A generic accessor for extracting an item from any sort of data blob.
*/
getItem: (data: any, index: number) => ?Item,
/**
* Determines how many items are in the data blob.
*/
getItemCount: (data: any) => number,
```
Changelog:
[Internal]
Reviewed By: lunaleaps
Differential Revision: D17863130
fbshipit-source-id: 40a1d57e3b4dd1e38c84d5907fe88f6b665287ae
Summary:
This is a tighter type that matches the actual prop as defined on line 100
Changelog:
[Internal]
Reviewed By: lunaleaps
Differential Revision: D17863133
fbshipit-source-id: 97f966ff13aa2ce36ef936a9a154fdd137191c6b
Summary: This change surprisingly fixes a Flow error because these types weren't compatible with the prop types these were passed into that had `info: `. Flow passes now though!
Reviewed By: lunaleaps
Differential Revision: D17863131
fbshipit-source-id: 094f1d97e96686d16bb69732b8a4b319492b5780
Summary: These comments existed for other components that had these props. Putting them here as this is the proper base of those other components. Also adding `ItemSeparatorComponent` as it is used in this component.
Reviewed By: lunaleaps
Differential Revision: D17860495
fbshipit-source-id: b7b60058d37e90699b28419af27d488bd46d3ebd
Summary: The instance type wasn't being set properly. Using AbstractComponent
Reviewed By: lunaleaps
Differential Revision: D17859988
fbshipit-source-id: 95e2098a7218afeaf3f6ee39ba2b69170ee2f54c
Summary: These need to be both optional and nullable to support spreading props like `<ScrollView {...props} />` where these types are optional in a parent component.
Reviewed By: lunaleaps
Differential Revision: D17859633
fbshipit-source-id: 093456d13ee041473a4605e62bf48b3510b49b8f
Summary: These are already defined as part of ViewProps. They don't need to be duplicated
Reviewed By: lunaleaps
Differential Revision: D17859553
fbshipit-source-id: c3de534526efd94c0a9ff2c772a4d92c6164815b
Summary:
`?React.Element<any>` allows passing in `undefined`, an invalid value to render into a React component. Changing these types to be `null | React.Element<any>`.
The issues that this caught were fixed in a previous diff.
Reviewed By: lunaleaps
Differential Revision: D17859220
fbshipit-source-id: 71438cb357b44bca0bf3437aea99ece99a616f7d
Summary:
Moving this to the class lets the first argument be typed as `Props<SectionT>` instead of `Props<SectionBase<any>>`. This is consistent with the structure of FlatList
Changelog:
[Internal]
Reviewed By: zackargyle
Differential Revision: D17841258
fbshipit-source-id: 3e0e6c2f6b21cbce0e662647cb43a012e062c4bc
Summary:
These props are unusupported by the component they were being passed to.
Changelog:
[Internal]
Reviewed By: zackargyle
Differential Revision: D17839765
fbshipit-source-id: 13c80a07da2026b61070ffc93f26194b979ee8fc
Summary:
Fixing up some of the FlowFixMes in VirtualizedList
Changelog:
[Internal]
Reviewed By: zackargyle
Differential Revision: D17839611
fbshipit-source-id: c763a799efca63fd7110cfaed87afde80995b8aa
Summary: backgroundColor must be passed in a style prop
Reviewed By: PeteTheHeat
Differential Revision: D17838513
fbshipit-source-id: 315ca144f63e67b0e9db5e705074974372db2f5e
Summary:
These props were pretty much all typos and thus didn't do anything.
My two choices are to change the code to do what it seems it *intends*, or keep the behavior the same. To de-risk, I'm preferring to keep the behavior the same.
Changelog:
[Internal]
Differential Revision: D17834712
fbshipit-source-id: 2f6b6376ff5936d4ad20291022c043ff7808bac4
Summary:
Ensure that on Android, only Scheduler and UIManagerBinder directly retain UIManager. These existing JSI function-retained lambdas will not have owning share_ptr refs to UIManager, just raw pointers.
By the time the VM deallocates all JSI objects and UIManagerBinding goes away, it should be able to deallocate UIManager synchronously when UIManagerBinding is deallocated. The VM should not be executing any of these JSI functions at that time, so this should ensure that the raw pointer access is safe.
This should also provide some extremely small perf wins, and make our memory model easier to reason about, since we're using shared_ptr less often and now `UIManager` is only owned in two places: Scheduler and UIManagerBinding.
So, there are now only two places where UIManager can be deallocated:
1) Hermes is deallocated first, and UIManagerBinding is deallocated before Scheduler. Scheduler holds onto UIManager. Scheduler later is deallocated and frees UIManager; this would cause a crash if it's not completed before Hermes is torn down, but we currently ensure that Scheduler is torn down before Hermes. This is fine but we should document this contract, or make Scheduler not own UIManager.
2) Scheduler is deallocated first, decrementing the shared_ptr to UIManager. Later Hermes is torn down, which deallocates UIManagerBinding via the JSI pointer table, which synchronously deallocates UIManager before Hermes has shut down. This is the desired outcome.
Changelog:
[[Internal]]
Reviewed By: shergin
Differential Revision: D17872586
fbshipit-source-id: cbb25b5cd025f5f8597dc7a66073fe64edc57aa8
Summary: To enable onScroll animations with Fabric's scrollView on iOS, we dispatch onScroll event to Paper's eventDispatcher as well as to Fabric's one. One we have a proper NativeAnimationDriver in place, we will get rid of this.
Reviewed By: shergin
Differential Revision: D17814260
fbshipit-source-id: f04faf59cdfd4ea5cede513388e30500b4cb2ad5
Summary: See https://eslint.org/docs/rules/no-useless-escape. Useless escapes can reflect a mismatch between the intended and actual effect of a backslash in a literal.
Reviewed By: rubennorte
Differential Revision: D17876784
fbshipit-source-id: 7641b1f2227b92e1e91469adc0d0d990a64109cf
Summary:
Setting minimal throttle to 1/60 causes dropped updates.
Let's take following example
Each frame is 16.666 MS.
Frame1: [________didScroll is called towards end of the frame_]
Frame2: [_didScroll is called towards beginning of the frame_________]
update from Frame 2 doesn't propagate because we set throttle to 16MS thinking that `onScroll` is called for each frame.
If Javascript specified value below 1/60 it is basically asking for every update.
Reviewed By: shergin, mdvacca
Differential Revision: D17829926
fbshipit-source-id: e7b07fd09581cd5053aa27a62cf6f6ddb2193783
Summary: Move RCTScrollEvent into separate file and export its header.
Reviewed By: shergin
Differential Revision: D17831709
fbshipit-source-id: f4ee4e09147ef5703b85a10238ddb6cdefdf05a5
Valentin Shergin
Samuel Susla
zhongwuzw
James Ide
Ramanpreet Nara
Eli White
Moti Zilberman
Sharon Gong
empyrical
Maksym Rusynyk
Tim Yung
Albert Sun
Joshua Gross