Using extended classes for props doesn't get picked up by react-refresh

[Zaid-Ajaj]

I have a curious case where I am using classes instead of plain old JS objects as input properties for my components.

Simple usage with classes like the following works as expected and maintains its state when I make changes to styling etc.

import { createElement, useState } from 'react' 
 
export class CounterProps {
    constructor(title) {
        this.title = title;
    }
}

export const Counter = (props) => {
  const [count, setCount] = useState(0)
  return createElement("div", null, 
    createElement("h1", null, props.title),
    createElement("h1", null, count), 
    createElement("button", { onClick: () => setCount(count + 1) }, "Increment")
  );
}

export const Counters = () => {
  return createElement("div", null, 
     createElement(Counter, new CounterProps("Page title")),
     createElement(Counter, new CounterProps("Another title"))
   );
}

However, once I extend CounterProps with anything then fast-refresh doesn't pick up the changes anymore:

export class Anything { }

export class CounterProps extends Anything {
    constructor(title) {
        super();
        this.title = title;
    }
}

I am using the following versions (on Windows):

• react-refresh v0.9.0
• @pmmmwh/react-refresh-webpack-plugin v0.4.3
• react v16.14.0
• webpack v4.44.2

[Zaid-Ajaj]

I just confirmed that removing export from the class definitions makes react-refresh work again! Also if these types were defined in another file and exported, then it all works out. Maybe React-refresh is detecting classes as function components because they are upper-cased?

[gaearon]

I would expect that classes without a render method shouldn’t be detected as components. But you should verify in react-refresh/runtime source. We could fix it there if that’s the issue.

[Zaid-Ajaj]

Thanks @gaearon for the quick response! I will have a look at the source code, it is in the main react repo if I remember correctly.

Fixing this would make a huge difference for Fable/React applications!

[Zaid-Ajaj]

I found this function called isReactClass:

function isReactClass(type) {
  return type.prototype && type.prototype.isReactComponent;
}

And tested it locally in the application itself, it correctly returns undefined for the class definitions I have above. It doesn't seem to be the cause of the error. I can confirm that the functions returns true when the class actually extends Component from react

[Zaid-Ajaj]

@gaearon I think the function isLikelyComponentType might be the culprit. Because it returns true for an empty class like this:

export class Anything { }

isLikelyComponentType(Anything) // true

The fix is to change this part:

if (ownNames.length > 1 || ownNames[0] !== 'constructor') {
  // This looks like a class.
  return true;
}

Into

if (ownNames.length >= 1 || ownNames[0] !== 'constructor') {
  // This looks like a class.
  // check whether the class has a render function
  const hasRenderFunction = ownNames.includes("render");
  return hasRenderFunction;
}

However, I couldn't verify that this fix actually solves the problem at hand :/ any pointers? Thanks in advance 🙇

[pmmmwh]

@gaearon I did some tests, and this is a bit tricky.

After transpilation, empty classes are de-sugared into this:

function _classCallCheck(instance, Constructor) {
  if (!(instance instanceof Constructor)) {
    throw new TypeError("Cannot call a class as a function");
  }
}

var Anything = function Anything() {
  _classCallCheck(this, Anything);
};

So, with respect to the current heuristic:

https://github.com/facebook/react/blob/e6a0f276307fcb2f1c5bc41d630c5e4c9e95a037/packages/react-refresh/src/ReactFreshRuntime.js#L674-L697

• It will fail the first condition (type.prototype.isReactComponent does not exist)
• It will fail the second condition (ownNames.length is 0 when de-sugared, 1 when kept intact, ownNames[0] is 'constructor' when not de-sugared)
• It will fail the third condition (type.prototype.__proto__ === Object.prototype since it is an empty class/function)

The main question here is whether there exist a heuristic to differentiate empty classes from functions (especially considering when de-sugared to ES5), not the other way around (detecting a render function exists within a class).

[Zaid-Ajaj]

@pmmmwh Is there anything I can help with in order to fix this issue? Any pointer would be great! 🙏

[pmmmwh]

@pmmmwh Is there anything I can help with in order to fix this issue? Any pointer would be great! 🙏

I am actually unsure if this can be solved when a de-sugared empty class is essentially the same as a plain function ... ?

[Zaid-Ajaj]

Could a babel-macro be implemented to rewrite the definition into something that can be detected by react-refresh?

I mean in my case is really that Anything is defined in another module and CounterProps extending it makes application fully refresh on changes, instead of the partial refresh.

Maybe there is a simpler way: I checked that making the classes as lower-case still causes the problem, maybe should react-refresh ignore these functions when they are lower-cased?

[pmmmwh]

Could a babel-macro be implemented to rewrite the definition into something that can be detected by react-refresh?

What do you have in mind?

Maybe there is a simpler way: I checked that making the classes as lower-case still causes the problem, maybe should react-refresh ignore these functions when they are lower-cased?

Can you create a reproducible case? I don't seem to be able to see this behaviour in a bare-bones setup.

A Sandbox to demonstrate this:

https://codesandbox.io/s/extend-class-react-refresh-23d00

[Zaid-Ajaj]

Thanks a lot for the sandbox example, I will try to replicate the configuration as close as possible to this working example and let you know of my findings!

[Zaid-Ajaj]

@pmmmwh I was able to make the smallest possible repo, it is in this repository Please have a look at it 🙏

Run the sample cloning locally and executing

npm install
npm run dev

Navigate to localhost:8080

Click around to change the state of the counters

Then update the content of the button from "Increment" to "Increment by 1" (inside Components.fs.js) and notice react-refresh performing a full page refresh. However, once you change this part

export class CounterProps extends Record {
    constructor(initialCount) {
        super();
        this.initialCount = (initialCount | 0);
    }
}

into

export class CounterProps {
    constructor(initialCount) {
        this.initialCount = (initialCount | 0);
    }
}

Then react-refresh works as expected and the state is preserved across re-renders