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

Make react-hot-loader@next a devDependency. #1261

Merged
merged 1 commit into from
Jun 1, 2017

Conversation

jspears
Copy link
Contributor

@jspears jspears commented May 31, 2017

...

@jsf-clabot
Copy link

jsf-clabot commented May 31, 2017

CLA assistant check
All committers have signed the CLA.

@skipjack
Copy link
Collaborator

skipjack commented Jun 1, 2017

@jspears good catch, I'll merge but note that this guide may go away or change significantly once #1251 is finished.

@skipjack skipjack merged commit b3b4d94 into webpack:master Jun 1, 2017
@rohannair
Copy link

rohannair commented Jun 14, 2017

This should not have been merged. react-hot-loader provides app-level code. Doing an npm install --production would break this dependency and cause issues in an application. Note the official docs for react-hot-loader: https://github.com/gaearon/react-hot-loader#installation

This PR should be reverted.

@skipjack
Copy link
Collaborator

Ah you're correct -- I think it's because of the AppContainer that must be imported and wrapped around the root. However, we removed this guide in #1251 and replaced it with a simpler one that links to things like react-hot-loader but doesn't try to replicate all the documentation for these external tools.

@rohannair
Copy link

Oh I didn't realize. All good then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants