Closed kelset closed 4 years ago
alloy
commented
4 years ago FYI In an effort to keep a good overview of things that need to be resolved for the next RC or final release; I’m going through all the open comments in this thread and hide all those that are out-of-date, about issues that were not introduced in v0.62.0 nor have a landed fix, or are otherwise not actionable.
mysport12
commented
4 years ago This issue has appeared whilst testing the new RC facebook/react-native#28098
https://github.com/facebook/react-native/pull/27618 seems to be the culprit. I too am seeing issues with modals on both iOS 13 and iOS 12 as described in the comments
jacobp100
commented
4 years ago RE @mysport12
Do we need to cherry pick these commits -
https://github.com/facebook/react-native/commit/ed11a12a7c831225d2725855e95bde1f93ea16b3 https://github.com/facebook/react-native/commit/66f89e2e3651cf518b2fbc134a7a09cbce94a140
mysport12
commented
4 years ago @jacobp100 Yes, that is what I am thinking. I don't think we need to pick https://github.com/facebook/react-native/commit/66f89e2e3651cf518b2fbc134a7a09cbce94a140 though since the original commit https://github.com/facebook/react-native/commit/7e8a18840fec9b51e2599809cf18830bc719d3f7 doesn't appear to be in 0.62.0-rc2
kelset
commented
4 years ago Thanks @mysport12 and @jacobp100 for your investigation, we'll make sure to cherry pick those reverts 👍 (or revert the original commit locally, depending on what's easier) really appreciated the help 🙇♂️
jacobp100
commented
4 years ago Ah sorry - I think the revert commit was meant to be https://github.com/facebook/react-native/commit/ed11a12a7c831225d2725855e95bde1f93ea16b3 . I think we do have the original commit in the RC (see https://github.com/react-native-community/releases/issues/157#issuecomment-583124811)
owinter86
commented
4 years ago Can we cherry pick this https://github.com/facebook/react-native/commit/6adba409e6256fd2dcc27a4272edcedae89927af as it fixes the autofocus glitch you see when using react-native screens and closes issue https://github.com/software-mansion/react-native-screens/issues/236
alloy
commented
4 years ago @mysport12 @jacobp100 Am I correct in summarising the back and forth that in the next RC we only need to pick https://github.com/facebook/react-native/commit/ed11a12a7c831225d2725855e95bde1f93ea16b3?
alloy
commented
4 years ago Regarding the issue with (new) Android apps not launching from CLI, need to bump the CLI package to v4.1.1, which contains this revert.
alloy
commented
4 years ago CLI issue that affects new watch mode and Flipper new reload/open dev menu buttons: react-native-community/cli#984 (both being shipped in
0.62).
@lucasbento It was unclear to me what action to take on this, can you elaborate?
alloy
commented
4 years ago v0.62.0-rc.3 was just released with all suggested changes picked 🎉
grabbou
commented
4 years ago No action needed from the CLI perspective, FWIW we will ship all the patches as, well... patch releases.
We just need to make sure CLI is stable before RN hits stable so that everyone gets the latest and we have an opportunity to bump it to the latest too.
Let's sync before we're about to do so.
kelset
commented
4 years ago Just FYI: we are aware that with RC3 you may be hitting some Android-side crashes, we are investigating it 💪
grabbou
commented
4 years ago I have confirmed Android works w/o issues on my machine.
Installing APK 'app-debug.apk' on 'Pixel_2_API_27(AVD) - 8.1.0' for app:debug
owinter86
commented
4 years ago @grabbou what about release version? I can successfully install and run android debug builds, but get this error when trying to do release version.
com.android.builder.testing.api.DeviceException: com.android.ddmlib.InstallException: INSTALL_FAILED_INVALID_APK: Package couldn't be installed in /data/app/REDACTED-9ssMp3TR_vtsbrqlt0Vtxw==: Package /data/app/REDACTED-9ssMp3TR_vtsbrqlt0Vtxw==/base.apk code is missing
grabbou
commented
4 years ago Same without issues. Might be something specific to your environment.
owinter86
commented
4 years ago Ok thanks, I will take a look. Seems like I can build fine with Hermes enabled (debug and release), but disabled, I get that error for release builds. I will go over the upgrade diff again.
owinter86
commented
4 years ago @grabbou invalidating caches seemed to do the trick, I have no issues on my end, can mark my comments as resolved.
hsjoberg
commented
4 years ago @grabbou which issue were you referring to? Android still crashes for me with the same error that I posted above. Debug or release doesn't matter. If I compile in Android Studio first as was suggested it works sometimes, but I cannot get it consistently to work, only maybe 1 in 10 times.
It works fine if I just return null without any element, but as soon as I add say a <Text>, it crashes.
From the log I see a lot of:
02-14 00:55:32.039 6005 6051 W unknown:ViewManagerPropertyUpdater: Could not find generated setter for class com.facebook.react.views.scroll.ReactScrollViewManager
etc, so perhaps it is related to everything not being loaded correctly.
jgreen210
commented
4 years ago For 0.62.0-rc.3, with the hermes engine enabled, fetch() leaks memory via the java Blobs holding the request bodies, causing an OutOfMemoryError if download enough data to fill up the java heap. This was fixed a while back with JavaScriptCore.
https://github.com/facebook/hermes/issues/164#issuecomment-591431693
mgcrea
commented
4 years ago @mgcrea AFAIK the FB team is going to cherry pick a few more Flipper-related commits/bugfixes before next RC, so hopefully in next it will be solved! Thanks for reporting anyway :)
Originally posted by @kelset in https://github.com/react-native-community/releases/issues/145#issuecomment-572559668
I'm still encountering https://github.com/facebook/react-native/issues/27845 / https://github.com/facebook/flipper/issues/834 (just created in case this was not tracked) with the latest rc.3, any update on whether this might be fixed before the release?
Thanks!
TheSavior
commented
4 years ago The Upgrade Helper results look much better for RC3 than it did for RC2. Nice work!
It looks like there are still a couple things that [stand out to me in this diff](upgrade helper for 0.61.5...0.62.0-rc.3).
android/app/build.gradle has a removal of entryFile: "index.js", but no matching addition. (On closer reading I see the new comment is saying this is automatic). Maybe this should be added as an inline comment on the removal in the upgrade-helper as well to make it even more obvious?android/app/build.gradle includes implementation "androidx.swiperefreshlayout:swiperefreshlayout:1.0.0" but that seems weird. Why does it need that? There are no other implementation calls adding other things. ios/RnDiffApp.xcodeproj/project.pbxproj changes. This request may be out of scope, but it would be great to add inline comments to that file explaining what users actually should change in their projects since they aren't supposed to edit those files directly. It looks like the among other things the organization name, development region, and some build settings changed.
safaiyeh
commented
4 years ago @TheSavior for the addition @passy mentioned something here: https://github.com/facebook/react-native/pull/28052#discussion_r378879042
This PR would remove the dependency from the app side but still part of ReactAndroid
vongohren
commented
4 years ago How can I know if this PR: https://github.com/facebook/react-native/pull/27523 will make it into this release? I have 0.61.5 now and im experiencing this
pvinis
commented
4 years ago In the latest diffs from 61 to 62, we still have the xcscheme files removed, when they shouldn't. I'm trying to figure out how to revert this. If anyone has some idea, please help. For now I just assume that it happened somewhere around the time the shareddata for xcode was added to the gitignore.
pvinis
commented
4 years ago Found it 3e4c5c09c37866ac3a711921a37136a392c618be. I'm making a PR to add them back in.
pvinis
commented
4 years ago Made the PR pointing to master, and I think it should be cherry-picked before the final 62.0. Here is is: https://github.com/facebook/react-native/pull/28198
mjmasn
commented
4 years ago Seeing a crash on startup with Hermes on Android after upgrading our app from 0.61.5 to 0.62.0-rc3. Switched back to JSC temporarily and it runs fine.
libc : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 9336 (mqt_js), pid 9217
robertying
commented
4 years ago Seeing a crash on startup with Hermes on Android after upgrading our app from 0.61.5 to 0.62.0-rc3. Switched back to JSC temporarily and it runs fine.
libc : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 9336 (mqt_js), pid 9217Full Crash log
Same here.
I am using react-native-navigation and I filed an issue to Hermes the other day: https://github.com/facebook/hermes/issues/189. But then I found out it only crashes in debug. Maybe you could try to build in release to see if that works for you too.
mjmasn
commented
4 years ago @robertying interesting. We're not using react-native-navigation so I guess it's something in common between that and the packages we're using...
I will try a release build in a minute :+1:
hsjoberg
commented
4 years ago It looks similar to my crash issue. It's probably not related to react-native-navigation (I'm using react-navigation), what happens if you just render null or empty fragment (<></>)? For me it won't crash anymore but as soon as I add one real element (say <Text>), it crashes.
Check my post here https://github.com/react-native-community/releases/issues/157#issuecomment-586032669
alloy
commented
4 years ago @mjmasn @robertying Can you try if this patch fixes the issue? https://github.com/facebook/react-native/commit/e4194a20a7 /cc @willholen
mjmasn
commented
4 years ago @robertying yep release build with Hermes runs fine :+1:
@hsjoberg it would seem like a different bug to yours as the debug build is working fine in JSC for me. Your stacktrace looks slightly different too.
@alloy can't make any promises as it's my last day on the job :laughing: I'll see what I can do though!
hsjoberg
commented
4 years ago @mjmasn I'll take a look again, I don't remember if mine crashed on JSC. I wrote that it didn't work in JSC either but maybe I made a mistake.
mjmasn
commented
4 years ago @alloy we're in luck, I was actually already set up for building RN from source :) Yep that patch LGTM :tada:
vongohren
commented
4 years ago @safaiyeh my comment was flooded here, can maybe @mjmasn answere this: https://github.com/react-native-community/releases/issues/157#issuecomment-592047355
mjmasn
commented
4 years ago @vongohren I'm nothing to do with Facebook / RN but the commit for that PR is https://github.com/facebook/react-native/commit/233fdfc014bb4b919c7624c90e5dac614479076f (see https://github.com/facebook/react-native/pull/27523#issuecomment-571799903)
It doesn't look like it's in this release branch though, no*. Maybe @alloy can cherry-pick it for you :pray:
* compare to e.g. https://github.com/facebook/react-native/commit/3b3c95b0170e60983eb6e89b910d100d08eee141 where it shows the release tags at the bottom of the blue commit message section
vongohren
commented
4 years ago @mjmasn ah okay, sorry, it was a random at for the most active here. But thank you very much for checking up and looking at it. Much appreciated 🎉🏅 Yeah it is a very strange bug and something that has been merged, so if ut can be cherry picked, @alloy that would be great! Maybe @safaiyeh has some more inputs on this.
hsjoberg
commented
4 years ago Can you try if this patch fixes the issue? facebook/react-native@e4194a2
I can confirm that facebook/react-native@e4194a2 works as well. :tada:
willholen
commented
4 years ago I have synced up with @mhorowitz, and would like to propose the following two picks:
Critical (and I see @hsjoberg already mentioned it above):
enableHermes: true will cause AwesomeProject to crash on load in debug mode (tested on both my emulator and physical device)Important:
There's also a nice-to-have issue that you can decide on:
An unresolved issue that may deserve a mention somewhere:
exclude "**/libhermes*.so" in packagingOptionsexclude "**/libhermes-executor-debug.so", exclude "**/libhermes-inspector.so", exclude "**/libjsc.so" . Unfortunately, this change will also break debug builds, and I’m not aware of a workaround that works in both cases.mjmasn
commented
4 years ago Not really a critical issue for release but this cli issue may affect existing apps with different package id suffixes per flavour/build type so I thought it'd be worth mentioning here.
robertying
commented
4 years ago I have synced up with @mhorowitz, and would like to propose the following two picks:
Critical (and I see @hsjoberg already mentioned it above):
- If not picked:
enableHermes: truewill cause AwesomeProject to crash on load in debug mode (tested on both my emulator and physical device)- If picked: AwesomeProject will load successfully
- Alternative workaround: None
Important:
- If not picked: JS debuggers will fail to attach to Hermes-based apps
- If picked: JS debuggers can see and attach to Hermes-based apps to debug JavaScript
- Alternative workaround: adding exclude "**/libhermes-executor-release.so" every time you want to debug Hermes, and removing it again every time you want to run the app in release mode
There's also a nice-to-have issue that you can decide on:
- If not picked: any errors loading JSC will result in an error message about Hermes, even if Hermes is not enabled
- If picked: When JSC fails to load, JSC load errors are surfaced to the user
- Alternative workaround: Explaining this on each relevant GitHub issue
An unresolved issue that may deserve a mention somewhere:
Since the last Gradle upgrade, react.gradle is unable to remove unused executors and support libraries ( facebook/react-native:react.gradle@
64720ab#L304 )
- This means that apps using JSC based APKs are 600kb larger than necessary per arch in release mode, which can be can be remedied with a manual
exclude "**/libhermes*.so"inpackagingOptions- Hermes based APKs are 600kb larger than necessary. This can be remedied with
exclude "**/libhermes-executor-debug.so", exclude "**/libhermes-inspector.so", exclude "**/libjsc.so". Unfortunately, this change will also break debug builds, and I’m not aware of a workaround that works in both cases.
I'm also seeing a regression that Android bundle (Google Play) version will crash on startup, which used to be a SoLoader issue. But I'm suspecting it has something to do with the issues above.
TheSavior
commented
4 years ago This commit fixes the missing xcscheme files. Let’s get this picked in: https://github.com/facebook/react-native/commit/a715decd2d3bcdab9537f3246c8398ad9869e94e
ovy9086
commented
4 years ago Not really a critical issue for release but this cli issue may affect existing apps with different package id suffixes per flavour/build type so I thought it'd be worth mentioning here.
Feel critical to me 😐 are there any possible solutions right now? I’m pretty sure there are a lot of users out there using this
mjmasn
commented
4 years ago @ovy9086 AFAIK it just prevents react-native run-android from opening the app automatically after install. You can still open the app manually.
ArrayZoneYour
commented
4 years ago > I have synced up with @mhorowitz, and would like to propose the following two picks: > > Critical (and I see @hsjoberg already mentioned it above): > > * [facebook/react-native@e4194a2](https://github.com/facebook/react-native/commit/e4194a20a7a8ba2bfe3f76f9fec38c04687bdf1e) > > * If not picked: `enableHermes: true` will cause AwesomeProject to crash on load in debug mode (tested on both my emulator and physical device) > * If picked: AwesomeProject will load successfully > * Alternative workaround: None > > Important: > > * [facebook/react-native@b8621f5](https://github.com/facebook/react-native/commit/b8621f5d303442ab78dc5d745cfc86a941d4737c) > > * If not picked: JS debuggers will fail to attach to Hermes-based apps > * If picked: JS debuggers can see and attach to Hermes-based apps to debug JavaScript > * Alternative workaround: adding exclude "**/libhermes-executor-release.so" every time you want to debug Hermes, and removing it again every time you want to run the app in release mode > > There's also a nice-to-have issue that you can decide on: > > * [facebook/react-native@65d3167](https://github.com/facebook/react-native/commit/65d3167a802b2ca04d4f05ff972c2d51765f1e0d) > > * If not picked: any errors loading JSC will result in an error message about Hermes, even if Hermes is not enabled > * If picked: When JSC fails to load, JSC load errors are surfaced to the user > * Alternative workaround: Explaining this on each relevant GitHub issue > > An unresolved issue that may deserve a mention somewhere: > > * Since the last Gradle upgrade, react.gradle is unable to remove unused executors and support libraries ( https://github.com/facebook/react-native/blob/64720ab14a68fba7ae5ac3100499365e3c84ba85/react.gradle#L304 ) > > * This means that apps using JSC based APKs are 600kb larger than necessary per arch in release mode, which can be can be remedied with a manual `exclude "**/libhermes*.so"` in `packagingOptions` > * Hermes based APKs are 600kb larger than necessary. This can be remedied with `exclude "**/libhermes-executor-debug.so", exclude "**/libhermes-inspector.so", exclude "**/libjsc.so"` . Unfortunately, this change will also break debug builds, and I’m not aware of a workaround that works in both cases.
I am using the workaround below for your last issue on 0.61, maybe it also works on your case
android/app/build.gradle
def taskName = getGradle().getStartParameter().getTaskRequests().toString()
packagingOptions {
// For Hermes, delete all the libjsc* files
exclude "**/libjsc*.so"
if (taskName.contains("Debug")) {
// Release libs take precedence and must be removed
// to allow debugging
exclude '**/libhermes-executor-release.so'
} else {
// Reduce size by deleting the debugger/inspector
exclude '**/libhermes-inspector.so'
exclude '**/libhermes-executor-debug.so'
}
}(edited by @kelset to collapse the quoted comment)
mweststrate
commented
4 years ago Fixed issue in 0.62.0-rc.3 where the iOS default template depends on a too old version of FlipperKit for iOS + react-native-flipper. Original PR: https://github.com/facebook/react-native/pull/28225, landing atm in D20249666
Edit: landed in https://github.com/facebook/react-native/commit/fbb94a30bc4ef667c9baadf55ca05d91a0be46fc
gorhom
commented
4 years ago added dark mode support for RCTPerfMonitor
https://github.com/facebook/react-native/commit/576ddfb3a84a5461679959f0d3f229a000dcea8d
alloy
commented
4 years ago 
timkuilman
commented
4 years ago 
I was just creating a release build with the latest rc and I noticed that it took quite a long time. It is processing Flipper and I'm wondering whether that is really necessary for a production build.
The getting started guide for Flipper mentions how to differentiate for Android between debug and production builds.
EDIT by @alloy:
The current version is
v0.62.0-rc.5and appears to be the golden one ✨TODOs: