-
Notifications
You must be signed in to change notification settings - Fork 404
0.61.x Discussion #140
Comments
This comment has been minimized.
This comment has been minimized.
should we just wait until master is green and re-cut after that? |
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. |
I have managed to dig deeper and fix a few tests. I am now working on fixing I will get back with the update, once fixed, we are good to go. |
This comment has been minimized.
This comment has been minimized.
I have picked them @fkgozali. The release is now being published. I have managed to resolve CircleCI issues by contacting their support team. |
That said, I have also unlocked this thread now that the release is going to be available really soon. |
This comment has been minimized.
This comment has been minimized.
@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) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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. |
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. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
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. |
@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. |
Shipping, closing and opening a new thread. |
…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
@grabbou will facebook/react-native@0cafa0f be included on next 0.61 patch release or just on 0.62? thanks! |
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? |
I'm still having issues with this on 0.61.1 (no HOC) / (didn't try HOC) |
|
As mentioned, version 0.61.1 does not fix the critical issue with |
@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. |
@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 |
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 |
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
The text was updated successfully, but these errors were encountered: