Skip to content

[flipper] Fix RNTester integration with Flipper #27482

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

Closed
wants to merge 2 commits into from

Conversation

passy
Copy link
Member

@passy passy commented Dec 11, 2019

Summary:
There was a reflective call to a non-existent class. It did, however,
exist in the template, so I copied it over from there and updated
the references accordingly.

Test Plan:
Built it and ran it. Works again with the latest Flipper desktop app.

Screenshot 2019-12-11 16 02 40

Summary:
There was a reflective call to a non-existent class. It did, however,
exist in the template, so I copied it over from there and updated
the references accordingly.

Test Plan:
Built it and ran it. Works again with the latest Flipper desktop app.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Dec 11, 2019
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.

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

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@mdvacca mdvacca left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!


public class ReactNativeFlipper {
public static void initializeFlipper(Context context, ReactInstanceManager reactInstanceManager) {
if (FlipperUtils.shouldEnableFlipper(context)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it documented how to enable flipper? By default it will not work in debug mode, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should work by default in debug builds. :) This is just an additional check to ensure that this is a) an IS_INTERNAL_BUILD b) not an end-to-end test (but that's FB-specific) and c) is on the main process as apps with multiple processes may not map native libraries into their address space.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @passy in 68bf0e3.

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 Dec 16, 2019
@rickhanlonii rickhanlonii mentioned this pull request Dec 18, 2019
30 tasks
rickhanlonii pushed a commit that referenced this pull request Jan 29, 2020
Summary:
There was a reflective call to a non-existent class. It did, however,
exist in the template, so I copied it over from there and updated
the references accordingly.
Pull Request resolved: #27482

Test Plan:
Built it and ran it. Works again with the latest Flipper desktop app.

![Screenshot 2019-12-11 16 02 40](https://user-images.githubusercontent.com/9906/70637975-02405580-1c30-11ea-9fec-23860c59cdb6.png)

Reviewed By: rickhanlonii

Differential Revision: D18933530

Pulled By: passy

fbshipit-source-id: 4515d7baaad9a9fff9a4b66e1cbf8a75889e6e45
@hramos hramos deleted the flipper-rntester-fix branch February 25, 2020 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants