Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

0.61.x Discussion #140

Closed
grabbou opened this issue Aug 20, 2019 · 92 comments
Closed

0.61.x Discussion #140

grabbou opened this issue Aug 20, 2019 · 92 comments
Labels
pre-release rc Release candidate

Comments

@grabbou
Copy link
Member

grabbou commented Aug 20, 2019

Conversation on this thread are limited to 0.61.x issues and cherry-pick requests from commits that are already on master.

An example of a good such request is a bug fix for a serious issue that has been merged into master but did not make the 0.61.x cut.

In other words, if you cannot point to a particular commit on master, then your request likely belongs as a new issue in http://github.com/facebook/react-native/issues.

If the commit you request to cherry-pick is a complicated port, you will be asked to create the PR to the 0.61-stable branch yourself, in order to ensure that the process proceeds smoothly

@react-native-community react-native-community locked and limited conversation to collaborators Aug 20, 2019
@elicwhite

This comment has been minimized.

@kelset kelset added pre-release rc Release candidate labels Aug 21, 2019
@kelset
Copy link
Member

kelset commented Aug 21, 2019

should we just wait until master is green and re-cut after that?

@grabbou
Copy link
Member Author

grabbou commented Aug 21, 2019

I haven't been able to get any updates regarding CI failures for the past few days and it's not clear when they will be fixed.

After discussing it on the #releases channel, we have decided to ship it "as is" because it works locally and it's an RC.

It's a requirement to cherry-pick CI fixes before we go stable, but we want to ship some of the new features out the door.

@grabbou
Copy link
Member Author

grabbou commented Aug 21, 2019

I have managed to dig deeper and fix a few tests. I am now working on fixing use_framework! tests to make sure we ship a release that works 100%.

I will get back with the update, once fixed, we are good to go.

@fkgozali

This comment has been minimized.

@grabbou
Copy link
Member Author

grabbou commented Aug 27, 2019

I have picked them @fkgozali. The release is now being published. I have managed to resolve CircleCI issues by contacting their support team.

@react-native-community react-native-community unlocked this conversation Aug 27, 2019
@grabbou
Copy link
Member Author

grabbou commented Aug 27, 2019

That said, I have also unlocked this thread now that the release is going to be available really soon.

@gaodeng

This comment has been minimized.

@kelset kelset changed the title 0.61.x Discussons 0.61.x Discussion Aug 27, 2019
@brentvatne
Copy link

brentvatne commented Aug 28, 2019

@gaodeng - i didn't add android support to it yet but https://github.com/expo/react-native-appearance is also something you can use right now. it's that PR extracted to a lib that works on react-native 0.59+ (and maybe earlier, at least the parts w/o hooks)

@owinter86

This comment has been minimized.

@kelset

This comment has been minimized.

@owinter86

This comment has been minimized.

@kelset kelset pinned this issue Aug 28, 2019
@kelset
Copy link
Member

kelset commented Aug 28, 2019

yes, but ideally we'd prefer to have here only the link to the issue; that way the conversation/investigation/debugging of that issue can happen over there and not here.

@elicwhite
Copy link
Contributor

This commit which I believe is included in 0.61 made a breaking change to TouchableNativeFeedback and the revert will need to get picked once it lands. I'm leaving on vacation today before RN.EU and might not be able to flag the actual commit.

@BrendonSled

This comment has been minimized.

@kelset

This comment has been minimized.

@BrendonSled
Copy link

@kelset facebook/react-native#26258

@xzilja

This comment has been minimized.

@gaearon

This comment has been minimized.

@gaearon

This comment has been minimized.

@sebqq

This comment has been minimized.

@gaearon

This comment has been minimized.

@owinter86

This comment has been minimized.

@elicwhite

This comment has been minimized.

@dulmandakh

This comment has been minimized.

@mikebouwmans

This comment has been minimized.

@kelset

This comment has been minimized.

@spyshower

This comment has been minimized.

@mikebouwmans

This comment has been minimized.

@marcorm

This comment has been minimized.

@mikebouwmans

This comment has been minimized.

@kelset

This comment has been minimized.

@mikebouwmans

This comment has been minimized.

@jeffreyffs

This comment has been minimized.

@grabbou
Copy link
Member Author

grabbou commented Sep 24, 2019

As I said in my previous comment, we need to resolve all remaining bugs and issues before this release gets stable.

The software-mansion/react-native-reanimated#381 still remains unresolved and I wanted to get it fixed before this gets stable. Reasons: the library is quite popular and we should always look out for the effects of the release on the community libraries.

@grabbou

This comment has been minimized.

@grabbou
Copy link
Member Author

grabbou commented Sep 24, 2019

I just spoke to @kmagiera on a side and he said that fix for software-mansion/react-native-reanimated#381 is on the way and is scheduled to land in a matter of day(s).

Since it will land in the userland and does not affect the release modes, I think we are good to safely ignore this error and proceed with 0.61.

@grabbou
Copy link
Member Author

grabbou commented Sep 24, 2019

@gaodeng, we are working on keeping the cherry-picks to critical issues only. I identified your commit as a non-critical addition that can be safely released under 0.62, which we will soon begin to work on.

@grabbou
Copy link
Member Author

grabbou commented Sep 24, 2019

Shipping, closing and opening a new thread.

@grabbou grabbou closed this as completed Sep 24, 2019
grabbou pushed a commit to facebook/react-native that referenced this issue Sep 25, 2019
…26495)

Summary:
This change restores the possibility of injecting custom root views via ReactAcitivtyDelegate. It has been used by react-native-gesture-handler library in order to replace default root view with a one that'd route touch events to gesture-handler internal pipeline.

The regression happened in d0792d4 where new `ReactDelegate` was introduced to provide support for rendering react native views in both Android fragments and activities. As a part of that change the logic responsible for creating root view has been moved from `ReactActivityDelegate` to `ReactDelegate` rendering `ReactActivityDelegate.createRootView` unused – that is there is no code path that leads to this method being called. Instead `ReactDelegate.createRootView` method has been added which now plays the same role. The custom root view injection was relying on overwriting that method and hence the method is no longer referenced overwriting it has no effect. Following the logic migration out of `ReactActivityDelegate` into `ReactDelegate` we could potentially now start overwriting methods of `ReactDelegate`. However when working with Android's activities in React Native we can only provide an instance of `ReactActivityDelegate` and in my opinion it does not make too much sense to expose also a way to provide own instance of `ReactDelegate`.

The proposed fix was to route `ReactDelegate.createRootView` to call `ReactActivityDelegate.createRootView` and this way regaining control over root view creation to `ReactActivityDelgate`. The change of the behavior has been implemented by subclassing `ReactDelegate` directly from `ReactActivityDelegate` and hooking the aforementioned methods together. Thanks to this approach, the change has no effect on `ReactDelegate` being used directly from fragments or in other scenarios where it is not being instantiated from `ReactActivityDelegate`.

This fixes an issue reported in software-mansion/react-native-gesture-handler#745 and discussed on 0.61 release thread: react-native-community/releases#140 (comment)

## Changelog

[Internal] [Fixed] - Allow for custom root view to be injected via ReactActivityDelegate
Pull Request resolved: #26495

Test Plan:
1. Run RNTester, take layout snapshot, see the react root view being on the top of view hierarchy.
2. Run gesture-handler Example app (or some other app that overwrites ReactActivityDelegate.createRootView method), on layout snapshot see custom root view being used.

Differential Revision: D17482966

Pulled By: mdvacca

fbshipit-source-id: 866f551b8b077bafe1eb9e34e5dccb1240fa935e
@felippepuhle
Copy link

@grabbou will facebook/react-native@0cafa0f be included on next 0.61 patch release or just on 0.62? thanks!

@ASchwad
Copy link

ASchwad commented Sep 25, 2019

According to the new release of react-native 0.61.1 they fixed something with react-native-gesture-handler. Is the HOC still required to use?

@bneigher
Copy link

I'm still having issues with this on 0.61.1 (no HOC) / (didn't try HOC)

@dulmandakh
Copy link
Contributor

@piuholo
Copy link

piuholo commented Sep 30, 2019

As mentioned, version 0.61.1 does not fix the critical issue with react-native-gesture-handler.

@kmagiera
Copy link
Member

@piuholo can you please describe in a little bit more details what the "critical issue" is? Which OS does it affect, which version of OS and RN. Preferably if you could make a simple repo from react-native init with gesture handler that we could run to see the issue that would be very helpful.

@piuholo
Copy link

piuholo commented Sep 30, 2019

@kmagiera In my case, it was the issue described here

software-mansion/react-native-gesture-handler#320

that had resurfaced on iOS after upgrading to RN 0.61.X from 0.60.X. However, I was able to work around the problem with the import statement found in the same issue thread.

@kelset
Copy link
Member

kelset commented Sep 30, 2019

Folks, sorry to bother you but there is a dedicated issue for 0.61.1 here.

Also, please always don't report issues directly on these issues but just comment linking to an issue from the main repo - it helps not losing track of them

@react-native-community react-native-community locked as resolved and limited conversation to collaborators Sep 30, 2019
@kelset kelset unpinned this issue Sep 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pre-release rc Release candidate
Projects
None yet
Development

No branches or pull requests