FredKSchott / snowpack

ESM-powered frontend build tool. Instant, lightweight, unbundled development. ✌️
https://www.snowpack.dev
MIT License
19.48k stars 921 forks source link

[BUG] HMR breaks with React class components #3323

Open DouglasDev opened 3 years ago

DouglasDev commented 3 years ago

Bug Report Quick Checklist

Describe the bug

When using React Class components with styled components, both React Fast Refresh and HMR break entirely and the browser doesn't update at all when you save a file.

To Reproduce

  1. clone and run this repo: https://github.com/DouglasDev/fastRefreshBreaks
  2. when you change the text in the component named "Updates.jsx" and save, the browser will update the text as expected.
  3. when you change the text in the component named "DoesntUpdate.jsx" and save, the browser does not update at all.

Expected behavior

It should update.

DouglasDev commented 3 years ago

Update: I found a partial workaround (works inconsistently): if all class components are wrapped with the withTheme higher-order component, fast refresh starts to work again. This makes me wonder if this is a bug in styled-components. Also, I noticed that in my previous example, the updated files are actually being generated correctly in the snowpack dist folder, but for some reason bubbled is set to false and the browser never loads the new files.

DouglasDev commented 3 years ago

Update 2: I figured out that the following code is not being injected into the file:

import * as  __SNOWPACK_HMR__ from '../_snowpack/hmr-client.js';
import.meta.hot = __SNOWPACK_HMR__.createHotContext(import.meta.url);
import * as __SNOWPACK_ENV__ from '../_snowpack/env.js';
import.meta.env = __SNOWPACK_ENV__;

I also figured out that if I render the class component inside a function component in the same file and then export the function component, the above code gets injected correctly and HMR starts working again. It looks like snowpack is having trouble telling that class components should use HMR. I might attempt a fix if someone can point me in the right direction as to where in the snowpack code base the bug would likely originate from.

DouglasDev commented 3 years ago

After further investigation, I've found other cases where the HMR code isn't being injected: files where the only component is being wrapped with the redux connect function and files with only components generated by styled components. Perhaps it's a more general bug related to React components generated by npm libraries.

DouglasDev commented 3 years ago

Also, I tried making the same components in create react app and they refresh correctly, so it's definitely a problem with snowpack or one of it's plugins.

DouglasDev commented 3 years ago

I've updated the title of this issue since figuring out that there isn't a problem with styled-components or react-refresh. I've done quite a bit of testing and I've found that they both appear to work correctly. The real problem is that when a class component is changed, the change should bubble up to it's parent component (in another file) but doesn't if there is a function component in the same file. Here is a minimal example that shows this:

import React, { Component } from 'react';

function A({children}){
    return <div>{children}</div>
}

export default class DoesntUpdate extends Component{
    constructor(props){
        super(props)
    }
    render(){
        return <A>not updating when changed</A>
    }
}

It seems that snowpack sees function A and assumes that this entire file can be hot reloaded when in fact it can't since the exported component is a class component and so needs to be reloaded from scratch. I think I'm getting closer to finding the source of the bug but any guidance would be greatly appreciated. Thanks.