Summary:
To ensure greater type safety, we want to generate some cpp tests for the fromRawValue conversions
This diff adds support to generate Tests.cpp along with the `buck test` targets itegrated into the rn_codegen rule automatically. The tests just `assert(true, true)` as a starting point
Reviewed By: fkgozali
Differential Revision: D14739493
fbshipit-source-id: fc9dea64ea31e6af7d997aebc54cfd459d48bf4f
Summary: This is removing packages and libraries from the repo. Any modified buck files simply change the redirect targets to something more appropriate (no logic actually changed)
Differential Revision: D14950721
fbshipit-source-id: 6c14f827b76ca1dbaf83dcb983930f362c6a27d4
Summary:
Second attempt at adding border properties to `RCTImageView`.
Previous attempt can be found at D14875673 which was reverted.
1. `UIImageView` is no longer laid out in in `layoutSubviews`.
2. `updateWithImage` was not being called, I instead of calling `self.image = image` I was calling `_imageView.image = image` directly which skips calling `updateWithImage`. This meant that image's rendering mode was not changed to `template`.
Reviewed By: shergin
Differential Revision: D14934103
fbshipit-source-id: b74c692f9f8ad520ef1f9c70ec4b4aa68b868cd4
Summary:
If you call into a Java method (from C++ using JNI) that raises an exception, the JNI call won't actually raise a C++ error. Instead, the `JNIEnv` will record the pending Java exception and the C++ will continue executing as normal. This is bad because the next time you call into JNI, the app will actually crash, unless you explicitly cleared the exception using `JNIEnv::ExceptionClear()` before the JNI call.
With respect to TurboModules, we need to make sure that RedBoxes show up whenever a native methods raise an exception. We also don't want the app to crash when a JNI method call fails because of a raised exception. Therefore, in this diff, I raise a C++ exception if `JNIEnv::ExceptionCheck()` is true.
Reviewed By: mdvacca
Differential Revision: D14738540
fbshipit-source-id: 4c3063aa93ae7aef025bd2dab6b45059bb8fb409
Summary:
If the return type of a TurboModule method is `Promise`, the infra should create a `com.facebook.react.bridge.Promise` object and pass it as the final argument of the TurboModule Java method call. The Java TurboModule method can then do some work asynchronously and either resolve or reject the promise at some point in time.
**Note:** I stacked a diff for error handling on top of this one.
Reviewed By: mdvacca
Differential Revision: D13653156
fbshipit-source-id: 4c30c3223ad8f47c6ba7f1236527aaced01c8ae8
Summary: If some class or category override/implement one of those methods, it implies that subclasses must call super for them. `NS_REQUIRES_SUPER` allows to enforce this constraint.
Reviewed By: JoshuaGross
Differential Revision: D14896804
fbshipit-source-id: a481f16e1f7ab901d70deb6486f2ca1cf19dd8d7
Summary:
Previously we could call `invalidateLayer` twice during a mounting transaction (one for update-props, one for update-layout-metrics), that was sub-optimal.
Now, with `finalizeUpdates:` we can do it only once. That should save us some CPU cycles.
Reviewed By: JoshuaGross
Differential Revision: D14896802
fbshipit-source-id: b6572fb2a4fa48a12b1d1c29144cd7c67f6cff80
Summary: We use this pattern already and seems we use it more. Those two functions introduce a "semantical" wrappers for this context, so now there is no need to think which exact `__bridge ***` qualifier we should use, so it's much less error-prone.
Reviewed By: JoshuaGross
Differential Revision: D14896800
fbshipit-source-id: 85b86bfcefdad5aff0375e7172769df86c001506
Summary:
Turns out that storing and using ContextContainer in custom subclasses is a huge pain. At the same time seems that a lot of custom components need some DI instrument, so we need this instrument anyway.
Moving stuff from the template to the base class should also help with codesize a bit.
Reviewed By: JoshuaGross
Differential Revision: D14921356
fbshipit-source-id: 4dbb961fe32bd66c73513d7e053bbed229860a31
Summary:
The new method is being called right after all update methods were called for a particular component view.
It is useful for performing updates that require knowledge of several independent aspects of the compound mounting change (e.g. props *and* layout constraints).
Reviewed By: JoshuaGross
Differential Revision: D14896801
fbshipit-source-id: 485d8c97d81d7a2ad7a7afc209094c328da6ef3c
Summary:
@public
This allows short methods defined in class declarations to occupy a single line.
The change makes class declarations more readable.
Reviewed By: SidharthGuglani
Differential Revision: D14950012
fbshipit-source-id: 1321949475184181c6cceb86613f730e430763e2
Summary:
Would like feedback from the community as this may not be the best solution for all
I would like to restrict (or paginate) the fling of a horizontal ScrollView when `snapToInterval` is set. This is not currently possible with `pagingEnabled`, since the pagination works only when items are the entire width of the ScrollView.
This implementation simply restricts the predicted `targetOffset` found from the `x` velocity and replaces it with the offset when the pan gesture ended.
To get pagination working, I may paginate based on the interval by calculating the offset delta from the beginning of the gesture to current offset and restricting the scrolling behavior to the `snapToInterval`. If this is preferred, I can update this PR or make a new one, but wanted to start a discussion since it seems like there are many in the community that would like this feature #21302 .
[General] [Added] - add prop `disableIntervalMomentum` to disable the predictive scrolling behavior of horizontal ScrollViews
Pull Request resolved: https://github.com/facebook/react-native/pull/24045
Differential Revision: D14939754
Pulled By: sahrens
fbshipit-source-id: 26be19c47dfb8eed4d7e6035df53a77451e23081
Summary:
For a small subset of Android devices, the following line of code caused the device to crash
```
output.transferFrom(input, 0, Long.MAX_VALUE);
```
According to the [Java docs](https://docs.oracle.com/javase/1.5.0/docs/api/java/nio/channels/FileChannel.html), this is what the function does.
> Transfers bytes into this channel's file from the given readable byte channel.
An attempt is made to read up to count bytes from the source channel and write them to this channel's file starting at the given position. An invocation of this method may or may not transfer all of the requested bytes; whether or not it does so depends upon the natures and states of the channels. Fewer than the requested number of bytes will be transferred if the source channel has fewer than count bytes remaining, or if the source channel is non-blocking and has fewer than count bytes immediately available in its input buffer.
Hence, using `Long.MAX_VALUE` seemed to be the standard way to transfer all bytes from one channel to the other.
However, it appeared that for some reason on a subset of old Android devices, the device tries to allocate `count` bytes of memory before transferring the bytes. Obviously, this caused a crash because no device has that much memory. This was fixed transferring bytes using a 1MB buffer.
Differential Revision: D14921778
fbshipit-source-id: 7fa46e10c656e23ae7d5679c72b278188f09ad0a
Summary:
This showcases SampleTurboModule usage in RNTester. Notes:
* iOS only for now, and you must use cocoapods version.
* You cannot use Chrome debugger when loading this specific example.
As illustrated in the example, the callsite should access `NativeSampleTurboModule` to access the native side.
{F155901711}
Reviewed By: cpojer
Differential Revision: D14932537
fbshipit-source-id: a733e1cd3b642b9e572d5ac6347f4775d495578a
Summary: This sets up RCTSampleTurboModule (and other variants) in RNTester when built with cocoapods. There's no call site yet though. And RNTester.xcodeproj doesn't support it.
Reviewed By: cpojer
Differential Revision: D14932535
fbshipit-source-id: db8eafd6777cbec8f3592dafdccbdd7cf44e38bc
Summary:
This provides various versions of SampleTurboModule, that are:
* compatible with existing NativeModule
* TurboModule compliant
Variants:
* RCTSampleTurboModule (traditional objc module)
* RCTSampleTurboCxxModule (objc++ module using CxxModule)
* SampleTurboModule (pure C++ impl of a TurboModule, no ObjC)
As noted in some files, they need to be codegen'ed based on the `NativeSampleTurboModule.js` (Flow type). The codegen script is not yet usable in OSS (we'll work on it some time in H2 2019). For now, these files need to be manually synced with Flow type.
Reviewed By: cpojer
Differential Revision: D14932539
fbshipit-source-id: fb887192384e5e6e4dff4cac68b4e037a4783cd9
Summary:
For CocoaPods variant only: install TurboModule binding so that sample modules can start using it. This commit only installs `global.__turboModuleProxy` - no sample module is provided.
Note: RNTester.xcodeproj will NOT have TurboModule enabled, due to complication in the .xcodeproj setup (doable, but maybe for some other time...)
To test:
```
console.error(global.__turboModuleProxy == null ? 'BOO' : 'YAY!');
```
Saw `YAY!` in RNTester pod version.
Reviewed By: cpojer
Differential Revision: D14932536
fbshipit-source-id: 3dc083da9154ec320ce6789ec7f2cef5a08fd6a7
Summary: This is to prepare for custom JS runtime binding installation. Note: AppDelegate needs to become .mm
Reviewed By: cpojer
Differential Revision: D14932538
fbshipit-source-id: 30f32223cc405d0cace9e3076b5a2a1d8cc9e3b2
Summary:
This adds https://github.com/mysticatea/abort-controller to polyfill [AbortController](https://developer.mozilla.org/en-US/docs/Web/API/AbortController). This is used to cancel requests when using `fetch`.
This also updates `event-target-shim` to 5.0 to make sure we only have one version of this dependency. This updates required adding a polyfill for `console.assert` which is used by the new version. I made one based on https://github.com/gskinner/console-polyfill/blob/master/console.js#L74.
The polyfill is very small, especially since we already use `event-target-shim` so I think it makes sense to include in core.
Depends on #24418 so that the fetch polyfill supports the `signal` parameter.
Fixes#18115
[General] [Added] - Add support for cancelling fetch requests with AbortController
Pull Request resolved: https://github.com/facebook/react-native/pull/24419
Differential Revision: D14912858
Pulled By: cpojer
fbshipit-source-id: 8a6402910398db51e2f3e3262f07aabdf68fcf72
Summary:
Closes: https://github.com/facebook/react-native/issues/24016
React Native 0.57 introduced cross-platform `accessibilityRole` and `accessibilityStates` props in order to replace `accessibilityComponentType` (for android) and `accessibilityTraits` (for iOS). With #24095 `accessibilityRole` and `accessibilityStates` will increase, receiving more options, which seems to be a good moment to remove deprecated props.
Remove deprecated `accessibilityComponentType` and `accessibilityTraits` props.
[General] [Removed] - Remove accessibilityComponentType and accessibilityTraits props
Pull Request resolved: https://github.com/facebook/react-native/pull/24344
Reviewed By: rickhanlonii
Differential Revision: D14842214
Pulled By: cpojer
fbshipit-source-id: 279945e503d8a23bfee7a49d42f5db490c5f6069
Summary:
`setupDevtools.js` is accessing `AppState.currentState` without checking its availability. In environments where 1) `__DEV__ == true`, and 2) no `RCTAppState` native module is provided thus resorting to `MissingNativeAppStateShim`, this will result in an exception:
```Cannot use 'AppState' module when native 'RCTAppState' is not included in the build. Either include it, or check 'AppState'.isAvailable before calling any methods.```
(Interestingly, `MissingNativeAppStateShim.currentState` did have a [default `null` value](118e88393e (diff-305b5180aa6ccc876ede6767de1fbfc4R192)) that was [later removed](a93b7a2da0 (diff-305b5180aa6ccc876ede6767de1fbfc4R186)).)
**Update**: Following cpojer's suggestion of reverting a93b7a2da0. Title also updated to reflect this.
[General] [Fixed] - Remove MissingNativeRCTNetworkingShim; revert MissingNativeAppStateShim
Pull Request resolved: https://github.com/facebook/react-native/pull/24380
Differential Revision: D14932658
Pulled By: cpojer
fbshipit-source-id: aef7ca566b3b8660eaed74a8ba3b6b0117b1200c
Summary:
We were using four edges for margin, padding and border. This diff changes the array size in YGLayout for margin, padding, border to reduce YGNode size and corresponding changes while we are setting values in YGLayout.
Reduces the YGNode size by 24 bytes
Reviewed By: davidaurelio
Differential Revision: D14892666
fbshipit-source-id: 94013d5183ee869901267c4c9941fd94fa05d848
Summary:
This PR fixes#24229.
Seems currently `opacity` props for Text is being applied twice (one for text color and one for the whole view). This PR disables applying the prop to the text.
[CATEGORY] [TYPE] - Fixed double applying opacity prop for Text
Pull Request resolved: https://github.com/facebook/react-native/pull/24435
Differential Revision: D14932795
Pulled By: cpojer
fbshipit-source-id: f9280fc75f788424cb5f1e42d2e79efdb354d645
Summary: Create views with props in one call instead of two. Backwards-compatible.
Reviewed By: shergin
Differential Revision: D14846424
fbshipit-source-id: cb53225579089f7e51d4e9d1fc9fc2e331a994c1
Summary:
`[RCTBridge setUp]` and `[RCTBridge invalidate]` execute asynchronously and concurrently. Therefore, it's not safe to call one method after the other, as we do in `[RCTBridge reload]`.
In this test, we create a bridge, and immediately reload it. Initializing the bridge causes the JS bundle to execute. Invalidating the bridge causes the jsThread to be terminated. If circumstances are correct, we could end up trying to executing the JS bundle after the jsThread has been terminated, which can lead to these assertions being triggered:
1. `RCTAssert(_jsThread, @"This method must not be called before the JS thread is created");` in `ensureOnJavaScriptThread:`.
2. `RCTAssert(_jsMessageThread != nullptr, @"Cannot invoke completion without jsMessageThread");` in `enqueueApplicationScript:url:onComplete:`.
```
- (void)testUnderlyingBridgeIsDeallocated
{
RCTBridge *bridge;
__weak id batchedBridge;
autoreleasepool {
bridge = [[RCTBridge alloc] initWithBundleURL:_bundleURL moduleProvider:nil launchOptions:nil];
batchedBridge = bridge.batchedBridge;
XCTAssertTrue([batchedBridge isValid], @"RCTBridge impl should be valid");
[bridge reload];
}
RCT_RUN_RUNLOOP_WHILE(batchedBridge != nil)
XCTAssertNotNil(bridge, @"RCTBridge should not have been deallocated");
XCTAssertNil(batchedBridge, @"RCTBridge impl should have been deallocated");
// Wait to complete the test until the new bridge impl is also deallocated
autoreleasepool {
batchedBridge = bridge.batchedBridge;
[bridge invalidate];
bridge = nil;
}
RCT_RUN_RUNLOOP_WHILE(batchedBridge != nil);
XCTAssertNil(batchedBridge);
}
```
To verify that this race is real, patch: P62410422. This adds an artificial delay in the `[RCTCxxBridge start]` method, which makes it so that the bridge is invalidated and the js thread is destroyed before we start executing the jsBundle.
I think a proper solution to this problem would require some bit of restructuring of `[RCTCxxBridge invalidate]` and `[RCTCxxBridge start]` to either:
1. Force `[RCTCxxBridge invalidate]` to wait for `[RCTCxxBridge start]` to complete and vice versa.
2. Make it safe to interleave execution of `[RCTCxxBridge start]` and `[RCTCxxBridge invalidate]`.
I tried the first approach using two semaphores: `_startSem(1)` and `_invalidateSem(0)`. When you start executing the code inside `[RCTCxxBridge start]`, you `semWait(_startSem)`. When you stop executing the code inside `[RCTCxxBridge start]` (which could happen in another thread at some later point in time), you `semSignal(_invalidateSem)`. Likewise, when you start executing `[RCTCxxBridge invalidate]`, you `semWait(_invalidateSem)` and when you stop executing the code inside `[RCTCxxBridge invalidate]` you `semSignal(_startSem)`. This way, invalidates always wait for starts to finish, and starts always wait for invalidates to finish. But considering all the concurrency involved in these methods, this is hard to get right.
The second approach seems possible. You could keep locks for the shared data, and create critical sections whever you want to access that data. I didn't actually try to implement this approach though.
Given that we're going to elminate the Bridge anyway, and that this race condition practically only occurs when you reload imediately after initializing the bridge (which can only really be done programmatically), I think it's fine to just disable the test for now. One other thing I considered was making the current thread sleep for some time after we created the bridge in the test. The reason why I'm hesitant to implement this approach is that it would slow down the execution of the test suite and still wouldn't guarantee that we don't hit this race condition. Ultimately, our infra might end up disabling these tests again.
Reviewed By: shergin
Differential Revision: D14909121
fbshipit-source-id: d7d441c3e2f0ad59182c8c7e23740be4ac4cf83c
Summary:
There are cases where native modules will try to reference other native modules during startup, which can causes a deadlock on `RCTModuleData::_instanceLock`.
My system was in a state where the deadlock would repro 100% so I could actually debug it a bit, but really this whole system is very fragile and needs to die in a fire.
I tried a recursive lock and it was not sufficient - there are definitely issues with multiple threads invoking at the same time and then calling `RCTUnsafeExecuteOnMainQueueSync`...
Reviewed By: fkgozali
Differential Revision: D14863583
fbshipit-source-id: 8c0d062353595a4ed3871aa9135950bc57983907
Summary:
Consolidate JavaScript tests from open source into a single script that can be executed by both Circle CI and Sandcastle, the internal Facebook CI.
[General] [Changed] - Switch internal and external CI to use the same script when running JavaScript tests
Pull Request resolved: https://github.com/facebook/react-native/pull/24422
Reviewed By: cpojer
Differential Revision: D14895773
fbshipit-source-id: d428929cc4e5219e02f5920259e08e0b3d24874c
Summary:
This diff replaces all MountItem classes with a bunch of static C functions that do the same job as classes did.
Seems, originally we overestimated the complexity of MountItem classes and that they ended up being notably trivial. Now, maintaining that even longer would mean paying for abstractions and allocations that we don't really need and writing a lot of tedious code.
Besides that, the one particular change that will be introduced in the coming diffs is not particularly fit very well in the existing class-based model.
This change also should save us many hundreds of allocations and atomic counters bumps, so maybe we can get a millisecond-or-two win.
This diff does not introduce any practical behavioral/logical changes in the mounting layer.
Reviewed By: mdvacca
Differential Revision: D14893764
fbshipit-source-id: 6f1247923ae36f29c12a7d358e2d496cf6c3e298
Summary:
`RCTImageView` is now a subclass of `RCTView` and includes `UIImageView` as it's subview.
This enables the use of `borderRadius`, `borderWidth`, `borderColor` properties and all of their derivatives.
Possible problem:
Now `RCTImageView` is backed by two views (`RCTView` + `UIImageView`), not a single one. That's 4 `CALayers`. Possible workaround would be to insert the image directly into `self.layer.contents`.
Reviewed By: RSNara
Differential Revision: D14875673
fbshipit-source-id: 594b2cd1ddffc6627566e07983c6d8f0b37dc2bb
Summary: This fixes a minor bug where the original props (like for styling) got dropped when the system falls back to UnimplementedView.
Reviewed By: mdvacca
Differential Revision: D14898906
fbshipit-source-id: 4a07952ceac66e491a5c0bc1ffd99f21438cda31
Summary: Similar to iOS, here we provide the basic impl of unimplemented view.
Reviewed By: mdvacca
Differential Revision: D14895706
fbshipit-source-id: 9053edfb2175b370d9070b6921794dbcafa1f37a
Summary: The changes made in D14811733 introduced a regression in React Native's open source iOS tests.
Reviewed By: fkgozali
Differential Revision: D14895096
fbshipit-source-id: 4307b094e3bc94e9f703b0271a95a9b318bcd49a
Summary:
Currently, `RCTLocalAssetImageLoader` only support directory of `Library` and `App bundle`, actually, we need to support other directories like `tmp` or `Documents`. Otherwise, the local image load in `tmp` or `Documents` would be handled by `NSURLSession`, we don't need that.
<img width="405" alt="image" src="https://user-images.githubusercontent.com/5061845/54188126-0ba66100-44ea-11e9-9a7b-0f721100e9be.png">
[iOS] [Fixed] - Enhance search of directories when load local asset image
Pull Request resolved: https://github.com/facebook/react-native/pull/23857
Differential Revision: D14894136
Pulled By: shergin
fbshipit-source-id: 26361cd952a423467be9af9a84a80100d868776b
Summary:
Motivation is following - I'm sure many people encountered this because it has been like this for a long time.
1 . you're developing something on android, HMR and dev mode is enabled
2 . you go to dev settings, you disable dev mode because you want to see how something behaves
3 . you reload the app because that's what is required for the change to take effect
4 . you wait for the bundle to be compiled and served, and when that is done, you get an error message about HMR not being a registered callable module - because HMR is not available when `__DEV__ === false` (todo screenshot)
this fixes the described case by checking if HMR is enabled and dev mode disabled when reloading (step 3) and disables HMR in that case.
this also fixes the case when dev mode is disabled and without knowing it, you try to enable HRM (will enable both dev hmr and dev mode).
[Android] [Changed] - improve developer experience around Dev mode and HMR interop
Pull Request resolved: https://github.com/facebook/react-native/pull/24377
Differential Revision: D14890695
Pulled By: cpojer
fbshipit-source-id: 95b6ff4131c6d05a32aadd09a9d5ed11f602122c
Summary:
There is a problem rendering text shadows on iOS. If the offset of the text shadow is `{width:0,height:0}`, the shadow does not display. This prevents you from representing a light directly above the text. This occurs because a text shadow only renders if the offset is a non-zero CGRect `{width:0,height:0}`.
My change checks `textShadowRadius` instead. If `textShadowRadius` is not nan then the user is rendering a text shadow. There are no situations to render a shadow without `textShadowRadius` making it a good variable to check.
This PR fixes this stale issue: https://github.com/facebook/react-native/issues/17277
[iOS] [Fixed] - Text shadow now displays when the textShadowOffset is {width:0,height:0}
Pull Request resolved: https://github.com/facebook/react-native/pull/24398
Differential Revision: D14890768
Pulled By: cpojer
fbshipit-source-id: a43b96a4a04a5603eede466abacd95c010d053e5
Summary: Running animations sometimes fail in Android. we are ignoring those failures temporarly
Reviewed By: fkgozali
Differential Revision: D14884510
fbshipit-source-id: 66d6113e12b1bd67e8bcc564943b423825b4cea6
Summary: This diff adds extra debug information in the mounting manager
Reviewed By: shergin
Differential Revision: D14817456
fbshipit-source-id: 5619c94eb76cdc20f5d7767f1aa4263e63f8d021
