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

Create React App + Hot Reloading not working? #1188

Closed
tomasruud opened this issue Oct 29, 2018 · 19 comments
Closed

Create React App + Hot Reloading not working? #1188

tomasruud opened this issue Oct 29, 2018 · 19 comments
Labels

Comments

@tomasruud
Copy link

Current behavior
When I try to use react-styleguidist with CRA, without changing the config, it seems like hot reloading are not working properly. Whenever I update my code, I am able to see the source refresh in the inspector, but nothing changes on the page. However, when I add

dangerouslyUpdateWebpackConfig: (c) => {
  c.cache = false
  return c
}

to my styleguide-config file, it's working as expected.

To reproduce
Here are my current set of dependencies and scripts

"scripts": {
  "format": "prettier --write src/**/*",
  "start": "react-scripts start",
  "build": "react-scripts build",
  "test": "react-scripts test --env=jsdom",
  "styleguide": "styleguidist server --open"
},
"dependencies": {
  "multi-download": "^2.0.1",
  "normalize.css": "^8.0.0",
  "prop-types": "^15.6.2",
  "react": "^16.6.0",
  "react-dom": "^16.6.0",
  "react-redux": "^5.1.0",
  "react-router-dom": "^4.3.1",
  "redux": "^4.0.1",
  "redux-thunk": "^2.3.0",
  "soundcloud": "^3.3.0",
  "spinkit": "^1.2.5"
},
"devDependencies": {
  "prettier": "^1.14.3",
  "react-scripts": "^2.0.5",
  "react-styleguidist": "^8.0.2"
}

I'm using css modules which comes with CRA.

Here is an example component:

import React from 'react'
import PropTypes from 'prop-types'

import styles from './Accordion.module.css'

const Accordion = ({children}) => (
  <ul className={styles.accordion}>{children}</ul>
)

Accordion.propTypes = {
  children: PropTypes.node.isRequired
}

export default Accordion

Expected behavior
I would expect this to work straight out of the box.

@sapegin
Copy link
Member

sapegin commented Oct 29, 2018

Could you make a demo project?

There's not much details about the cache option in the docs: https://webpack.js.org/configuration/other-options/#cache — I suspect it may share some cache between Styleguidist and CRA, but it's only a guess.

@tomasruud
Copy link
Author

tomasruud commented Oct 30, 2018

Yes, you can check out my project here https://github.com/tomasruud/downcloud

@malyzeli
Copy link

malyzeli commented Nov 20, 2018

Hello,

HMR really does not work by default, and actually it is not related to CRA, the same thing happens in clean project as well.

At first I thought it is caused by stateless functional components syntax, because I was sure that I used Styleguidist year ago with class components and it did work well. So I tested all possible component definitions and saw it is not related to actual syntax.

I created minimal example project so anybody can look further into it.

I can also confirm that setting dangerouslyUpdateWebpackConfig in styleguide config file as noted by @tomasruud enables hot reloading.

Edit: Seems it is caused by some webpack caching bug. HMR works again when cache: false option is added to webpackConfig section. Actualy there is no need to use dangerouslyUpdateWebpackConfig. I'm not sure what are the consequences of that. Thanks to @tomasruud for pointing me in right direction anyway.

Example project link: https://github.com/malyzeli/styleguidist-issue-1188-example

@tomasruud
Copy link
Author

tomasruud commented Nov 20, 2018

I would assume the webpackConfig and dangerouslyUpdateWebpackConfig options pretty much does the same thing when you're only changing the caching option? webpackConfig is probably safer to use though, but in CRA you need to use the dangerouslyUpdateWebpackConfig.

dangerouslyUpdateWebpackConfig is needed with CRA.

@sapegin
Copy link
Member

sapegin commented Nov 20, 2018

Feel free to send a pull request with a fix 👾

@malyzeli
Copy link

This problem is not related to CRA, please consider reopening #1206 and mark this issue as duplicate instead.

You can read the argumentation in my last comment

@seanlaff
Copy link

seanlaff commented Dec 22, 2018

Is the dangerouslyUpdateWebpackConfig workaround still working for you guys? @tomasruud I checked out your downcloud repo and can't get HMR working there. However @malyzeli I was able to get HMR working with your non-CRA repo.

Im on CRA 2.1.1 with styleguidist 8.0.6 and it does not seem to work for me either.

The HMR network requests do not even fire in my case, so I think it could be different problem than just the caching.

Im seeing the same behavior in the styleguidist v9 beta branch as well

@malyzeli
Copy link

@seanlaff I'm sorry, never used CRA so I can't help you with that..

@tomasruud Since maintainers of this repo ignore request to reopen #1206, would you mind removing CRA from name of this issue? I believe claiming explicitly that HMR does not work in general (which is apparently true) could attract more attention and hopefully got it fixed sooner.

I'm surprised that not many people noticed.. Maybe experienced developers able to fix it are not using CRA? :D

@seanlaff
Copy link

seanlaff commented Dec 23, 2018

@malyzeli Hot Reload works as I'd expect in your https://github.com/malyzeli/styleguidist-issue-1188-example repo (I'm on mac mojave), so I think the designation of CRA is correct for this issue.

I dug in some more and realized that as soon as the styleguidist webpage loads in a CRA repo, this message is output to the web console

The development server has disconnected.
Refresh the page if necessary.

webpackHotDevClient.js:65

which explains why I don't see updates. Not sure what causes the disconnect yet.

@seanlaff
Copy link

seanlaff commented Dec 23, 2018

I think I've found the problem. It would appear that Styleguidist HMR is broken by webpack-dev-server > 3.1.10 for both CRA and non-CRA rrepos.

The reason it appeared to be working in @malyzeli repo was because their yarn.lock had webpack-dev-server pinned 3.1.10, however if you install styleguidist in a fresh repo, you'll get 3.1.12 since styleguidist specifies ^3.1.10.

@tomasruud
Copy link
Author

Is the dangerouslyUpdateWebpackConfig workaround still working for you guys? @tomasruud I checked out your downcloud repo and can't get HMR working there. However @malyzeli I was able to get HMR working with your non-CRA repo.

Im on CRA 2.1.1 with styleguidist 8.0.6 and it does not seem to work for me either.

The HMR network requests do not even fire in my case, so I think it could be different problem than just the caching.

Im seeing the same behavior in the styleguidist v9 beta branch as well

@seanlaff I've not been working on the project that much lately, so I don't know to be honest. But I would assume the issue is still there.

@seanlaff I'm sorry, never used CRA so I can't help you with that..

@tomasruud Since maintainers of this repo ignore request to reopen #1206, would you mind removing CRA from name of this issue? I believe claiming explicitly that HMR does not work in general (which is apparently true) could attract more attention and hopefully got it fixed sooner.

I'm surprised that not many people noticed.. Maybe experienced developers able to fix it are not using CRA? :D

@malyzeli If this was a problem for all users of Styleguidist, more people would probably have noticed. Which in turn would have produced more activity on this issue and yours as well. I would assume in your case, that it's a rare edge case that by chance matches my CRA issue.

I've found the problem. Styleguidist HMR is broken by webpack-dev-server > 3.1.10 for both CRA and non-CRA rrepos.

The reason it appeared to be working in @malyzeli repo was because their yarn.lock had webpack-dev-server pinned 3.1.10, however if you install styleguidist in a fresh repo, you'll get 3.1.12 since styleguidist specifies ^3.1.10.

In my project webpack-dev-server is locked at 3.1.9 for react-scripts, and 3.1.10 for styleguidist, so this might be true. Were you able to get it to work by updaing webpack-dev-server @seanlaff ?

@seanlaff
Copy link

seanlaff commented Dec 23, 2018

Looking at the updates to dev-server, I suspect this is might be what caused it webpack/webpack-dev-server#1603 ... although that MR would lead me to believe I'd see invalid host errors, so perhaps it could be something else.

@seanlaff
Copy link

Here's my workaround. It works by short-circuiting the header check that webpack-dev-server added. This poses a security risk.

styleguide.config.js

module.exports = {
  dangerouslyUpdateWebpackConfig(config) {
    if (!config.devServer) {
      config.devServer = {};
    }
    config.devServer.disableHostCheck = true;
    return config;
  }
};

This leads me to believe webpack/webpack-dev-server#1603 is in-fact the root cause, but I still don't have a good explanation of why we don't see the invalid host errors.

@stale
Copy link

stale bot commented Mar 23, 2019

😴 This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week without any further activity. Consider opening a pull request if you still have this issue or want this feature.

@stale stale bot added the wontfix label Mar 23, 2019
@stale stale bot closed this as completed Mar 30, 2019
@wzup
Copy link

wzup commented Jun 4, 2019

@malyzeli

HMR works again when cache: false option is added to webpackConfig section.

No, it does not work

@malyzeli
Copy link

malyzeli commented Jun 12, 2019

@wzup

No, it does not work

It's been half a year, of course everything is different today - that's JavaScript, dude!

@UseMuse
Copy link

UseMuse commented Mar 19, 2020

@wzup
its work for me

import React from 'react'
import ReactDOM from 'react-dom'
import App from './App'

const rootEl = document.getElementById('root')

ReactDOM.render(
  <App />,
  rootEl
)

if (module.hot) {
  module.hot.accept('./App', () => {
    const NextApp = require('./App').default
    ReactDOM.render(
      <NextApp />,
      rootEl
    )
  })
}

@garg10may
Copy link

Was this resolved? It's still not working

@LordSeb
Copy link

LordSeb commented Jan 5, 2021

(not related to react-styleguidist but might help)

For my broken hot-reload on CRA it worked updating to versions:

"react": "^16.10.0",
"react-dom": "^16.10.0"

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

No branches or pull requests

8 participants