Skip to content
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

[Android ]Fix debugging on android for 0.63 #29204

Closed
wants to merge 1 commit into from
Closed

[Android ]Fix debugging on android for 0.63 #29204

wants to merge 1 commit into from

Conversation

devon94
Copy link
Contributor

@devon94 devon94 commented Jun 24, 2020

Summary

Currently on react native 0.63-rc.0 and 0.63-rc.1 enabling debugging throws an exception. It looks like something may have been missed in unregistering JSDevSupport in this commit c20963e

crash

This should fix #28746 and #29136

Changelog

[Android] [Fixed] - Fix crash when enabling debug

Test Plan

To recreate the bug:

npx react-native init RN063 --version 0.63.0-rc.1
react-native start
react-native run-android
Enable debug mode from react native dev menu

After this commit, the crash no longer occurs

non crash

@facebook-github-bot
Copy link
Contributor

Hi @devon94!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Jun 24, 2020
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 24, 2020
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 3005464

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@kesha-antonov
Copy link
Contributor

Yes, please!

@RSNara
Copy link
Contributor

RSNara commented Jul 6, 2020

Whoops. Thanks for the fix! 😅

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@RSNara has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @devon94 in 8c42c01.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 7, 2020
grabbou pushed a commit that referenced this pull request Jul 7, 2020
Summary:
Currently on react native 0.63-rc.0 and 0.63-rc.1 enabling debugging throws an exception. It looks like something may have been missed in unregistering JSDevSupport in this commit c20963e

![crash](https://user-images.githubusercontent.com/14797029/85500252-2acae400-b5b1-11ea-938a-674b55e649b2.gif)

This should fix #28746 and #29136

## Changelog
[Android] [Fixed] - Fix crash when enabling debug

Pull Request resolved: #29204

Test Plan:
To recreate the bug:

npx react-native init RN063 --version 0.63.0-rc.1
react-native start
react-native run-android
Enable debug mode from react native dev menu

After this commit, the crash no longer occurs

![non crash](https://user-images.githubusercontent.com/14797029/85500241-269ec680-b5b1-11ea-8cfe-85bfda4dd222.gif)

Reviewed By: TheSavior

Differential Revision: D22395406

Pulled By: RSNara

fbshipit-source-id: 046df77ae1c1de96870fb46f409d59e7d6a68c0d
@BrunoVillanova
Copy link
Contributor

Thank you very much for this fix!!
You are the man!

@TheWirv
Copy link
Contributor

TheWirv commented Jul 8, 2020

I've had this problem since upgrading to 0.63.rc1, but this commit/merge doesn't fix it.
Below is a screenshot that I just took, after building the app completely from scratch, after having fixed my RN files with this commit and creating a patch file as well:
I can give more input as needed.

Edit: Also, isn't the error message in the IllegalArgumentException wrong because it was copy-pasted?
Here I think it should be "In DebugCorePackage, ...

Here's the screenshot
Screenshot_1594229180

@devon94
Copy link
Contributor Author

devon94 commented Jul 8, 2020

@TheWirv Did you need to build the android source? If you choose to modify the code in node modules of or if you use RN source code without building from source, it won't apply native android changes. You can look at https://github.com/facebook/react-native/wiki/Building-from-source for reference and also this branch where I built the source code to test https://github.com/devon94/react-native/tree/fix-android-debug-release. Or you can also wait until a new RN release.

@TheWirv
Copy link
Contributor

TheWirv commented Jul 8, 2020

@TheWirv Did you need to build the android source? If you choose to modify the code in node modules of or if you use RN source code without building from source, it won't apply native android changes. You can look at https://github.com/facebook/react-native/wiki/Building-from-source for reference and also this branch where I built the source code to test https://github.com/devon94/react-native/tree/fix-android-debug-release. Or you can also wait until a new RN release.

@devon94 Thanks for the clarification. No, I don't need to build from source. I honestly didn't that this is necessary for Android source. I thought, I could just use patch-package to patch it, like I would with any other package. Then, I guess, I'll have to wait until the next RC/full release.

@BrunoVillanova
Copy link
Contributor

It is working now on v0.63.0 final, thanks to @devon94 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[0.63-rc.0] [Android] Could not find Native Module for JSDevSupport
8 participants