Skip to content

Reapply expo fixes with respect to bare workflow #20

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Reapply expo fixes with respect to bare workflow #20

wants to merge 5 commits into from

Conversation

sjchmiela
Copy link

@sjchmiela sjchmiela commented Nov 15, 2019 •
edited
Loading

Why

Fixes expo/expo#6262.

How

  • reformatted changes from sdk-36 so that the compare between commits looks as small as possible
  • reverted changes that will be done by ReactAndroidCodeTransformer when we import react-native-lab to android 6f62ba2
  • prepared react-native-lab for bare-expo and for ReactAndroidCodeTransformer by (3ba4045):
    • extracting getExponentActivityId so ReactAndroidCodeTransformer can replace the -1 with access to mDevSettings.mSettings.exponentActivityId which is added and exposed only by the transformer
    • reloading JS if expo-reloading fails (this would happen in bare-expo, what we want to do then is to just reload JS)
  • instead of commenting out stuff right in react-native-lab, I added labels to statements that should be removed so that ReactAndroidCodeTransformer deletes them when importing to expo repo. 4c65ae2

@sjchmiela sjchmiela requested review from brentvatne and ide November 15, 2019 10:37
@sjchmiela sjchmiela force-pushed the sdk-36-bare branch 2 times, most recently from 66a4d43 to 1157a21 Compare November 15, 2019 14:12
@sjchmiela
Copy link
Author

Ideally this pull request would be squashed into Brent's ce1bf40.

In my opinion the most readable way to review this PR is to look at the diff between last-but-one commit from sdk-36 and HEAD of this branch: https://github.com/expo/react-native/compare/sdk-36~1..sdk-36-bare This way we can verify that Brent's changes will be properly applied and ReactAndroid won't break in bare.

sjchmiela added a commit to expo/expo that referenced this pull request Nov 15, 2019
@sjchmiela
[tools] Delete labeled nodes when importing ReactAndroid, fixup with e…
740503f 
…xpo/react-native#20
@sjchmiela sjchmiela mentioned this pull request Nov 15, 2019
Merged
ide
ide requested changes Nov 16, 2019
View reviewed changes
Copy link
Member

@ide ide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there are many lines that don't need to change (ex: only the indentation changed).

@sjchmiela
Copy link
Author

The identation changes were introduced in ce1bf40, when code transformed by ReactAndroidCodeTransformer ended up here. This PR reverts those indentation changes, resulting with equally-big-diff. That’s why I suggested:

In my opinion the most readable way to review this PR is to look at the diff between last-but-one commit from sdk-36 and HEAD of this branch: https://github.com/expo/react-native/compare/sdk-36~1..sdk-36-bare This way we can verify that Brent's changes will be properly applied and ReactAndroid won't break in bare.

sjchmiela added a commit to expo/expo that referenced this pull request Nov 18, 2019
@sjchmiela
[tools] Delete labeled nodes when importing ReactAndroid, fixup with e…
60c026c 
…xpo/react-native#20
sjchmiela added a commit to expo/expo that referenced this pull request Nov 18, 2019
@sjchmiela
[android] Update ReactAndroid to bare-compatible react-native (#6286)
2afd270 
# Why

Fix for #6200 (comment).

# How

- expo/react-native#20
- updated `ReactAndroidCodeTransformer` so it removes labeled nodes
- ran `et urn`

# Test Plan

Expo client compiled ok.
@ide
Copy link
Member

ide commented Nov 20, 2019

@sjchmiela Could you merge this, and then (on your computer) do an interactive rebase and re-order + squash commits (and then force-push) so our commit history is clean again? 🙏

@sjchmiela
Copy link
Author

Oh, actually, I already did this! Sorry for forgetting to close this, I think I may have hoped it will be automatically closed by GitHub.

Left two commits out of those few:

@sjchmiela sjchmiela closed this Nov 21, 2019
@sjchmiela sjchmiela mentioned this pull request Nov 25, 2019
Closed
@ide ide deleted the sdk-36-bare branch April 25, 2020 08:35
gabrieldonadel pushed a commit that referenced this pull request Jul 31, 2025
@sammy-SC @facebook-github-bot
fix crash in view culling where differentiator creates create instruc…
34e9cba 
…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 #3: 0x00000001802b0144 libc++abi.dylib`abort_message + 128
    frame #4: 0x000000018029fe4c libc++abi.dylib`demangling_terminate_handler() + 296
    frame #5: 0x000000018006f220 libobjc.A.dylib`_objc_terminate() + 124
    frame #6: 0x00000001375d1964 INFRAFramework`meta_terminate() + 5468
    frame #7: 0x00000001802af570 libc++abi.dylib`std::__terminate(void (*)()) + 12
    frame #8: 0x00000001802b2498 libc++abi.dylib`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 32
    frame #9: 0x00000001802b2478 libc++abi.dylib`__cxa_throw + 88
    frame #10: 0x0000000180093904 libobjc.A.dylib`objc_exception_throw + 384
    frame #11: 0x0000000180e6999c Foundation`-[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 268
    frame #12: 0x000000031a3bcfc8 XPLAT_6_Framework`-[RCTComponentViewRegistry dequeueComponentViewWithComponentHandle:tag:] + 528
    frame #13: 0x000000031a3ccdec XPLAT_6_Framework`RCTPerformMountInstructions(std::__1::vector<facebook::react::ShadowViewMutation, std::__1::allocator<facebook::react::ShadowViewMutation>> const&, RCTComponentViewRegistry*, RCTMountingTransactionObserverCoordinator&, int) + 356
    frame #14: 0x000000031a3ccc7c XPLAT_6_Framework`-[RCTMountingManager performTransaction:]::$_1::operator()(facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) const + 80
    frame #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 #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 #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 #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 #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 #21: 0x000000032f219668 XPLAT_1_Framework`facebook::react::TelemetryController::pullTransaction(this=0x00000003f4680f00, willMount=0x000000016d4f0c98, doMount=0x000000016d4f0c78, didMount=0x000000016d4f0c58) const at TelemetryController.cpp:39:3
    frame #22: 0x000000031a3c5b28 XPLAT_6_Framework`-[RCTMountingManager performTransaction:] + 544
    frame #23: 0x000000031a3c5864 XPLAT_6_Framework`-[RCTMountingManager initiateTransaction:] + 456
    frame #24: 0x000000031a3c5240 XPLAT_6_Framework`__42-[RCTMountingManager scheduleTransaction:]_block_invoke + 308
    frame #25: 0x0000000131f81b84 BOTTOMFramework`__RCTExecuteOnMainQueue_block_invoke + 40
    frame #26: 0x000000018017c788 libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame #27: 0x0000000180197278 libdispatch.dylib`_dispatch_client_callout + 12
    frame #28: 0x00000001801b2fcc libdispatch.dylib`_dispatch_main_queue_drain.cold.7 + 24
    frame #29: 0x000000018018c1c4 libdispatch.dylib`_dispatch_main_queue_drain + 1184
    frame #30: 0x000000018018bd14 libdispatch.dylib`_dispatch_main_queue_callback_4CF + 40
    frame #31: 0x0000000180427fec CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12
    frame #32: 0x00000001804229f8 CoreFoundation`__CFRunLoopRun + 1920
    frame #33: 0x0000000180421e3c CoreFoundation`CFRunLoopRunSpecific + 536
    frame #34: 0x0000000190f62d00 GraphicsServices`GSEventRunModal + 164
    frame #35: 0x0000000185bcec98 UIKitCore`-[UIApplication _run] + 796
    frame #36: 0x0000000185bd3064 UIKitCore`UIApplicationMain + 124
    frame #37: 0x0000000115fbf0bc PRODUCTFramework`main + 200
    frame #38: 0x00000001114293d8 dyld_sim`start_sim + 20
    frame #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
Reviewers

@brentvatne brentvatne brentvatne approved these changes

@ide ide ide approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sjchmiela @ide @brentvatne