forked from facebook/react-native
-
Notifications
You must be signed in to change notification settings - Fork 147
Testci2 #3
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
Closed
Closed
Testci2 #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
/azp run |
No pipelines are associated with this pull request. |
/azp run |
No pipelines are associated with this pull request. |
tom-un
pushed a commit
to tom-un/react-native
that referenced
this pull request
May 21, 2019
…to match Android Summary: ## Overview This diff switches the RCTSlider onSlidingComplete event on iOS from a bubbling event to a direct (non-bubbling) event to match the non-bubbling type on android. Note that in this case these seems like a bug. I will still explain the motivation and reasoning as this will come up in future diffs. ## Changelog [Slider][BREAKING] Switch Slider onSlidingComplete event to a non-bubbling event on iOS to match Android ## Motivation: The motivation here is that when we codgen the view configs, we'll need to unify all of the events and props across platforms for components that have the same name on iOS and Android. In this case, the view configs (below) conflict for onSlidingComplete. On iOS this is under bubblingEventTypes, on Android this is under directEventTypes. We have code [here](https://fburl.com/3s1dahm2) in the react native renderer which ensures an event is not listed as both. ``` // iOS const SliderViewConfig = { bubblingEventTypes: { onSlidingComplete: { phasedRegistrationNames: { captured: 'onChangeCapture', bubbled: 'onChange' } } }, directEventTypes: { // None }, validAttributes: { // ... } }; ``` ``` // Android const SliderViewConfig = { bubblingEventTypes: { // None }, directEventTypes: { onSlidingComplete: { registrationName: 'onEventDirect' } }, validAttributes: { // ... } }; ``` ## Solutions There are three solutions to this issue: 1. Don't generate view configs 2. Rename the component on one platform 3. Make a breaking change in the event behavior on one platform to make it consistent across both platforms Here we've chosen option microsoft#3 Reviewed By: TheSavior Differential Revision: D15322304 fbshipit-source-id: ff1ab86efe9e2bc50fd3f7619e6760ab5c1c5090
NickGerleman
pushed a commit
to NickGerleman/react-native
that referenced
this pull request
Oct 3, 2020
Summary: This diff allows specifying the preemptive compilation threshold via CompileFlags. Users get a toggle in CLI and RuntimeConfig that allows choosing between 1. fully eager, 2. fully lazy, 3. use default thresholds ("smart"). The default is microsoft#3. However, since we use `-O` by default, it overrides any lazy compilation when using the `hermes` binary. ChangeLog: [Internal] Inspector updated to use new RuntimeConfig laziness control Reviewed By: tmikov Differential Revision: D23356463 fbshipit-source-id: 508b7b2e6a218346c69acfec97e7891e388f0e9b
NickGerleman
pushed a commit
to NickGerleman/react-native
that referenced
this pull request
Mar 1, 2021
Summary: This fix is still a little hypothetical. We have a few different JS errors that we're seeing with bridgeless mode that seem to be caused by Fabric trying to access `__fbBatchedBridge` from C++. I think what's happening is: 1. User encounters an unrelated JS error very early in rendering a new surface (possibly while the bundle is still loading?) 2. In release builds, BridgelessReactFragment handles the error by stopping the surface and rendering a retry button (actually, the surface is stopped in a bunch of places in BaseFbReactFragment, which might be why this is popping up now - I recently refactored that class to share more of its logic in bridgeless mode) 3. Fabric stops the surface by first checking to see if the custom binding `RN$stopSurface` exists; if not, it falls back to calling the registered callable module `ReactFabric`. I think microsoft#3 is where things are going wrong for bridgeless mode; if you call stopSurface before `RN$stopSurface` is installed (which happens when ReactFabric shim is required) then you'll fall back to the bridge version. My solution here is to instead rely on a flag set in C++ to determine whether we're in bridgeless mode, and then check to see if the stopSurface binding has been installed. If not, we just noop - if the ReactFabric shim hasn't been required, we probably don't actually have a React surface that needs to be stopped. At least, that's my current theory. We'll see if this actually works. Changelog: [Fixed][iOS] Fix an issue calling stopSurface in bridgeless mode before surface is started Reviewed By: mdvacca Differential Revision: D25453696 fbshipit-source-id: bff76675c43989101d0ba5ae0aba60089db230bf
amgleitman
added a commit
that referenced
this pull request
Aug 25, 2021
Fix Android Build Issues for RN64 Upgrade for patches through 2020-06-30
NickGerleman
pushed a commit
to NickGerleman/react-native
that referenced
this pull request
Nov 21, 2021
Summary: Previous iterations of this diff that were reverted: D30678341 (facebook@8009459), D30868627 (facebook@abd0f38). With the power of selects, we can move the base AppleTVOS flags into the regular base Apple flags. While I'm here, drop the third `p` in `get_application_apppletvos_flags()` as it's driving me insane. Note - This puts get_visibility_option() on all Apple builds. I believe this is the right thing to do as everything except macOS static libraries already do it, and it shouldn't affect binaries. Changelog: [Internal] Reviewed By: mzlee, jdonald Differential Revision: D31024459 fbshipit-source-id: be85296cda3612836c1c74632dc52f6e3f89512e
NickGerleman
pushed a commit
to NickGerleman/react-native
that referenced
this pull request
Nov 21, 2021
…ake microsoft#3) Differential Revision: D31024459 (facebook@0a46e5e) Original commit changeset: be85296cda36 fbshipit-source-id: ceb8e5a3cda57a40a876b07133d8021942308878
NickGerleman
pushed a commit
to NickGerleman/react-native
that referenced
this pull request
Jan 12, 2022
Summary: This diff adds `overflowInset` values in RN Android. These values are used to give an enlarged boundary of a view group that also contains all its children layout. Here is [the post](https://fb.workplace.com/groups/yogalayout/permalink/2264980363573947/) that discuss more on why this is useful. I steal the pic in that post here as TLDR: {F687030994} In the above case, we will get overflowInset for ViewGroup A as something like `top: 0, right: -20, bottom: -20, left: 0`. This has been added in the [Fabric core](https://fburl.com/code/f8c5tg7b) and [in IOS](https://fburl.com/code/vkh0hpt6). In Android, since we used to ignore all event coordinates outside of a ViewGroup boundary, this is not an issue. However, that caused unregistered touch area problem and got fixed in D30104853 (facebook@e35a963), which dropped the boundary check and made the hit test algorithm in [TouchTargetHelper.java](https://fburl.com/code/dj8jiz22) worse as we now need to explore all the child node under ReactRootNode. This perf issue is getting obvious when a view loads too many items, which matches our experience with "Hover getting slow after scrolling", "Hover getting slow after going back from PDP view", and "The saved list view (in Explore) is very fast (because it has very few components)" To fix this issue, I added the support to `overflowInset` to RN Android by 1. Sending the `overflowInset` values from Binding.cpp in RN Android as a separate mount instruction 2. Update `IntBufferBatchMountItem.java` to read the int buffer sent over JNI, and pass the `overflowInset` values to `SurfaceMountingManager.java` 3. Creating new interface `ReactOverflowViewWithInset.java` and extending the existing `ReactOverflowView.java` usages 4. Adding implementation of getter and setter for `overflowInset` in various views 5. Update `TouchTargetHelper.java` to read the values and check boundaries before exploring ViewGroup's children Note that in microsoft#3 I didn't change `ReactOverflowView.java` interface directly. I am concerned about backward compatibility issues in case this interface is being used in OSS. I suggest we deprecate it as we are not using it anymore in our code. Changelog: [Internal][Android] Reviewed By: JoshuaGross Differential Revision: D33133977 fbshipit-source-id: 64e3e837fe7ca6e6dbdbc836ab0615182e10f28c
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Mar 19, 2025
Summary: Noticed a couple bugs here, around a crash from assertion internal to the caching map which hashes on the AttributedString, that may or may not be related. 1. We are missing `baseTextAttributes` for both hashing and equality (which is mostly innocuous, but still wrong) 2. We were not hashing or comparing `textAlignVertical` 3. For equality, we were comparing parent shadow view tag and metrics, but for hashing, we were hashing the whole ShadowView. I think microsoft#3 could cause issues, since we could see different hash despite equality, which could break invariants. Changelog: [Internal] Differential Revision: D71500246
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Mar 19, 2025
Summary: Noticed a couple bugs here, around a crash from assertion internal to the caching map which hashes on the AttributedString, that may or may not be related. 1. We are missing `baseTextAttributes` for both hashing and equality (which is mostly innocuous, but still wrong) 2. We were not hashing or comparing `textAlignVertical` 3. For equality, we were comparing parent shadow view tag and metrics, but for hashing, we were hashing the whole ShadowView. I think microsoft#3 could cause issues, since we could see different hash despite equality, which could break invariants. Changelog: [Internal] Reviewed By: lunaleaps Differential Revision: D71500246
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Mar 21, 2025
Summary: Pull Request resolved: facebook#50147 Noticed a couple bugs here, around a crash from assertion internal to the caching map which hashes on the AttributedString, that may or may not be related. 1. We are missing `baseTextAttributes` for both hashing and equality (which is mostly innocuous, but still wrong) 2. We were not hashing or comparing `textAlignVertical` 3. For equality, we were comparing parent shadow view tag and metrics, but for hashing, we were hashing the whole ShadowView. I think microsoft#3 could cause issues, since we could see different hash despite equality, which could break invariants. Changelog: [Internal] Reviewed By: lunaleaps Differential Revision: D71500246 fbshipit-source-id: 462749d5ca10d10bf0dab88089253a2bb8e603fb
NickGerleman
pushed a commit
to NickGerleman/react-native
that referenced
this pull request
May 13, 2025
…tion for existing view (facebook#51294) Summary: Pull Request resolved: facebook#51294 changelog: [internal] Fix a crash where a node that is supposed to be culled doesn't get visited because culling context is not updated. The differentiator would generate a create instruction for a view that already exists. Stack trace for the crash: ``` * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT * frame #0: 0x0000000111740874 libsystem_kernel.dylib`__pthread_kill + 8 frame #1: 0x00000001117aa2ec libsystem_pthread.dylib`pthread_kill + 264 frame #2: 0x0000000180171ea8 libsystem_c.dylib`abort + 100 frame microsoft#3: 0x00000001802b0144 libc++abi.dylib`abort_message + 128 frame microsoft#4: 0x000000018029fe4c libc++abi.dylib`demangling_terminate_handler() + 296 frame microsoft#5: 0x000000018006f220 libobjc.A.dylib`_objc_terminate() + 124 frame microsoft#6: 0x00000001375d1964 INFRAFramework`meta_terminate() + 5468 frame microsoft#7: 0x00000001802af570 libc++abi.dylib`std::__terminate(void (*)()) + 12 frame microsoft#8: 0x00000001802b2498 libc++abi.dylib`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 32 frame microsoft#9: 0x00000001802b2478 libc++abi.dylib`__cxa_throw + 88 frame microsoft#10: 0x0000000180093904 libobjc.A.dylib`objc_exception_throw + 384 frame microsoft#11: 0x0000000180e6999c Foundation`-[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 268 frame microsoft#12: 0x000000031a3bcfc8 XPLAT_6_Framework`-[RCTComponentViewRegistry dequeueComponentViewWithComponentHandle:tag:] + 528 frame microsoft#13: 0x000000031a3ccdec XPLAT_6_Framework`RCTPerformMountInstructions(std::__1::vector<facebook::react::ShadowViewMutation, std::__1::allocator<facebook::react::ShadowViewMutation>> const&, RCTComponentViewRegistry*, RCTMountingTransactionObserverCoordinator&, int) + 356 frame microsoft#14: 0x000000031a3ccc7c XPLAT_6_Framework`-[RCTMountingManager performTransaction:]::$_1::operator()(facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) const + 80 frame microsoft#15: 0x000000031a3ccc20 XPLAT_6_Framework`decltype(std::declval<-[RCTMountingManager performTransaction:]::$_1&>()(std::declval<facebook::react::MountingTransaction const&>(), std::declval<facebook::react::SurfaceTelemetry const&>())) std::__1::__invoke[abi:ne190102]<-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&>(-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 40 frame microsoft#16: 0x000000031a3ccbc8 XPLAT_6_Framework`void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ne190102]<-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&>(-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 40 frame microsoft#17: 0x000000031a3ccb94 XPLAT_6_Framework`std::__1::__function::__alloc_func<-[RCTMountingManager performTransaction:]::$_1, std::__1::allocator<-[RCTMountingManager performTransaction:]::$_1>, void (facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&)>::operator()[abi:ne190102](facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 44 frame microsoft#18: 0x000000031a3cba1c XPLAT_6_Framework`std::__1::__function::__func<-[RCTMountingManager performTransaction:]::$_1, std::__1::allocator<-[RCTMountingManager performTransaction:]::$_1>, void (facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&)>::operator()(facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 44 frame microsoft#20: 0x000000032f219804 XPLAT_1_Framework`std::__1::function<void (facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&)>::operator()(this=0x000000016d4f0c78, __arg=0x000000016d4f0a10, __arg=0x000000016d4f0978) const at function.h:989:10 frame microsoft#21: 0x000000032f219668 XPLAT_1_Framework`facebook::react::TelemetryController::pullTransaction(this=0x00000003f4680f00, willMount=0x000000016d4f0c98, doMount=0x000000016d4f0c78, didMount=0x000000016d4f0c58) const at TelemetryController.cpp:39:3 frame microsoft#22: 0x000000031a3c5b28 XPLAT_6_Framework`-[RCTMountingManager performTransaction:] + 544 frame microsoft#23: 0x000000031a3c5864 XPLAT_6_Framework`-[RCTMountingManager initiateTransaction:] + 456 frame microsoft#24: 0x000000031a3c5240 XPLAT_6_Framework`__42-[RCTMountingManager scheduleTransaction:]_block_invoke + 308 frame microsoft#25: 0x0000000131f81b84 BOTTOMFramework`__RCTExecuteOnMainQueue_block_invoke + 40 frame microsoft#26: 0x000000018017c788 libdispatch.dylib`_dispatch_call_block_and_release + 24 frame microsoft#27: 0x0000000180197278 libdispatch.dylib`_dispatch_client_callout + 12 frame microsoft#28: 0x00000001801b2fcc libdispatch.dylib`_dispatch_main_queue_drain.cold.7 + 24 frame microsoft#29: 0x000000018018c1c4 libdispatch.dylib`_dispatch_main_queue_drain + 1184 frame microsoft#30: 0x000000018018bd14 libdispatch.dylib`_dispatch_main_queue_callback_4CF + 40 frame microsoft#31: 0x0000000180427fec CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12 frame microsoft#32: 0x00000001804229f8 CoreFoundation`__CFRunLoopRun + 1920 frame microsoft#33: 0x0000000180421e3c CoreFoundation`CFRunLoopRunSpecific + 536 frame microsoft#34: 0x0000000190f62d00 GraphicsServices`GSEventRunModal + 164 frame microsoft#35: 0x0000000185bcec98 UIKitCore`-[UIApplication _run] + 796 frame microsoft#36: 0x0000000185bd3064 UIKitCore`UIApplicationMain + 124 frame microsoft#37: 0x0000000115fbf0bc PRODUCTFramework`main + 200 frame microsoft#38: 0x00000001114293d8 dyld_sim`start_sim + 20 frame microsoft#39: 0x0000000111506b4c dyld`start + 6000 ``` Reviewed By: rubennorte Differential Revision: D74654157 fbshipit-source-id: 9181bcd28524c71d0ca4620bd630dc0baa172386
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Testing CI...