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

[HMR] Styles not updating with v2.2.1 #1225

Closed
bpedersen opened this issue Oct 9, 2017 · 20 comments
Closed

[HMR] Styles not updating with v2.2.1 #1225

bpedersen opened this issue Oct 9, 2017 · 20 comments

Comments

@bpedersen
Copy link

Version

2.2.1
babel-plugin-styled-components version: N/A

Reproduction

HMR is broken for styled component styles with upgrade from 2.2.0 to 2.2.1. I believe that the culprit may be #1069. Perhaps we could do a check if the code is running in dev environment and not cache the statics then.

Steps to reproduce

Run webpack-dev-server with HMR using the react-hot-loader plugin

Expected Behavior

When I change a style attribute in my styledComponent I would expect the style to change in the browser without a full page refresh. I did test that this is working in 2.2.0.

Actual Behavior

The style change does not happen until I do a full page refresh

@mxstbr
Copy link
Member

mxstbr commented Oct 9, 2017

Yep, very likely. Good catch, @schwers any idea how we can detect HMR? Should we turn off caching in development?

@tannerlinsley
Copy link

Ah ha! Just realized this is what's breaking styled-components hot reloading for react-static users. Looks like I'm gonna have to lock things down to 2.2.0 until it's fixed.

@schwers-zz
Copy link

schwers-zz commented Oct 9, 2017

@PiereDome sorry about that, if by chance its only affecting attrs usage I have a PR open for it #1219 (not sure why travis is failing but appveyor passes. edit: I was missing a changelog update, its all ✔️ now)

@mxstbr I'll have to try out something with HMR. I would think if the component is re-evaluated we wouldn't have to do anything. i.e. a new ComponentStyle should be created, which would recompute isStatic and its content hash; if its only attrs I think that patch will resolve. A dev check could work but right now it looks like we hardcode that in at build time so we might need to do some trickery to read in the user's env vars at runtime.

@bpedersen
Copy link
Author

@schwers I am not using attrs in my StyledComponents. It did seem to work when I was using the connect HOC from react-redux but not 100% sure that was the reason it was working. I can further try to narrow the working vs non-working components if you need me to.

@schwers-zz
Copy link

schwers-zz commented Oct 9, 2017 via email

@bpedersen
Copy link
Author

bpedersen commented Oct 10, 2017

Looks like if you have a function call inside the styledComponent the HMR will work. i.e.
const StyledComponent = styled(Component)` ${functionCall} background: red; `
const functionCall = () => ` padding: 7px; `

@kitten
Copy link
Member

kitten commented Oct 29, 2017

@mxstbr @schwers:

Should we turn off caching in development?

That sounds like a sensible solution to me 👍

@theKashey
Copy link
Contributor

As @PiereDome mentioned - HRM start working when you do have function inside style, ie style is not static.
RHL, on code update, will trigger componentWillReceiveProps, but component is sealed from any changes.

 componentWillReceiveProps(nextProps: { theme?: Theme, [key: string]: any }) {
      // If this is a staticaly-styled component, we don't need to listen to
      // props changes to update styles
      const { componentStyle } = this.constructor
      if (componentStyle.isStatic) {
        return
      }

So, just replace it by

      if (!module.hot && componentStyle.isStatic) {
        return
      }

PS: module exists in node.js and webpack environment, better to extract it as helper function.

@mxstbr
Copy link
Member

mxstbr commented Nov 1, 2017

Hmm, that could work but might break some folks if they don't use webpack. Maybe if they properly guard against it?

@theKashey
Copy link
Contributor

const isHMREnabled = () => typeof module !== 'undefined' && module.hot

Should I create a PR, or you can manage this by your own?

@mxstbr
Copy link
Member

mxstbr commented Nov 1, 2017

Please create a PR!

@cr0cK
Copy link

cr0cK commented Dec 21, 2017

Any news on this?

@theKashey
Copy link
Contributor

@cr0cK - was releases a month ago - v2.2.4

@cr0cK
Copy link

cr0cK commented Dec 22, 2017

Oh sorry. But it still doesn't work for me.
But I just noticed that it's all my HMR that is broken. Time to debug.

@elliothimmelfarb
Copy link

@cr0cK Did you figure it out? I'm having the same problem at v2.4.0.

@theKashey
Copy link
Contributor

@elliothimmelfarb - just post an example, and we will figure out which part is broken - HRM, RHL or Styled.

@elliothimmelfarb
Copy link

@theKashey
This is the error message I am getting in the devtools after a hot reload:

React Hot Loader: this component is not accepted by Hot Loader. 
Please check is it extracted as a top level class, a function or a variable. 
Click below to reveal the source location: 
 ƒ StyledComponent() {
        classCallCheck(this, StyledComponent);
        return possibleConstructorReturn(this, _ParentComponent.apply(this, arguments));
      }

Is that telling?

@theKashey
Copy link
Contributor

theKashey commented Jan 11, 2018

@elliothimmelfarb - it is about - StyledComponent could cause a state loss, as long it got updated.
Anyway - try to bump react-hot-loader yarn add --dev react-hot-loader(>4.6.0) and rerun.

@elliothimmelfarb
Copy link

@theKashey That did it! Thank you so much!

@hawkeng
Copy link

hawkeng commented Dec 20, 2018

I'm having this issue with storybook 4.0.11 and styled-components 3.4.10

Any ideas?

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

No branches or pull requests

9 participants