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

Allow both HMR clients and live reloading clients #11875

Closed
wants to merge 1 commit into from

Conversation

cpunion
Copy link
Contributor

@cpunion cpunion commented Jan 13, 2017

Explain the motivation for making this change. What existing problem does the pull request solve?

I almost preview on multi devices, sometimes I switch a device to HMR mode, and others are Live Reload mode, currently only the "HMR device" can update. I already used this patch a few months, and I think I'm not the only need it.

Prefer small pull requests. These are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.

Very small :-)

Test plan (required)

Test Steps:

  1. Open multi devices or emulators, switch one to HMR mode, and others are Live Reloading mode.
  2. Update your code.

Expected result:

  1. All devices can updated.
  2. The "HMR device" only update with HOT RELOADING experience.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @jeanlauliac and @davidaurelio to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 13, 2017
@cpunion
Copy link
Contributor Author

cpunion commented Jan 13, 2017

Aha, I forgot I created a pull request about this and it was closed.

See #6837

What's the status about that after a few months? can this be merged? I used it long time, but I must patch every version.

cpunion added a commit to cpunion/react-native that referenced this pull request Jan 21, 2017
@lacker
Copy link
Contributor

lacker commented Feb 9, 2017

The comments are a bit odd, usually you don't want comments in the source code to refer to "this commit" because it's totally unclear what commit that's referring to.

Is there any behavior that gets degraded with this pull request?

Can you add a test to ensure this functionality continues to work? The reloading stuff seems to break a lot so we could really use better tests there.

@shergin shergin added Android Platform: iOS iOS applications. labels May 25, 2017
@skevy
Copy link
Contributor

skevy commented Jun 8, 2017

Hi there!

The packager no longer lives in this repo. It now lives in https://github.com/facebook/metro-bundler. The reasoning for the split is explained here: #13976.

If you're still interested in pursuing changes to the packager for this PR, please open a PR on that repo! :)

Thanks!
-Adam

@skevy skevy closed this Jun 8, 2017
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. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants