new rule: forbid exporting React Components and non-components from the same module

[Alloyed]

According to the official react-fast-refresh docs:

• If you edit a module that only exports React component(s), Fast Refresh will update the code only for that module, and re-render your component. You can edit anything in that file, including styles, rendering logic, event handlers, or effects.
• If you edit a module with exports that aren't React components, Fast Refresh will re-run both that module, and the other modules importing it. So if both Button.js and Modal.js import Theme.js, editing Theme.js will update both components.
• Finally, if you edit a file that's imported by modules outside of the React tree, Fast Refresh will fall back to doing a full reload. You might have a file which renders a React component but also exports a value that is imported by a non-React component. For example, maybe your component also exports a constant, and a non-React utility module imports it. In that case, consider migrating the constant to a separate file and importing it into both files. This will re-enable Fast Refresh to work. Other cases can usually be solved in a similar way.

In react-native and webpack, this rule is implemented using a method that iterates over every export at runtime and checks if it is likely to be a component.

Could we statically check for the same thing to catch cases where this rule is violated?

side note: in typescript code, this rule could be relaxed to allow for type-only exports, such as interfaces, type aliases, or as const enums.

[ljharb]

That is a very absurd restriction imo. The typical structure I see and use is that a file default-exports a React component, and named-exports its propTypes, and indeed in TS code, types are named-exported along with that.

Is there any part of the ecosystem besides react-fast-refresh with this restriction?

[Alloyed]

That kind of structure would be legit under this rule: when you look at the compiled js for a typescript file, any kind of type export is going to be stripped out, which would include your propsType. The rule is more to catch cases like this:

export const myConstant = 100;

or, maybe more insidiously

export const myGlobalMutableState = new StateObject();

where we can't rely on the assumption that state is being managed by react.

re: ecosystem question, fast-refresh is the current approach to hot module reload in react. Any bundler for react native or react-dom that has HMR is using fast-refresh under the hood. Most of the time though, it's not a problem even if you break the rule: it just means that the bundler and ultimately the browser are going to need to make and load bigger chunks up until the point where you hit one of these edge cases that causes things to break down.

[ljharb]

No, that would not include my non-TS propTypes object, which remains at runtime intentionally.

[Alloyed]

oh, that's referring to the props-type library, gotcha. In that case, if you were to do that right now, then HMR would not work for that component. Instead, the chunk of code that would recursively update would be the highest level component that didn't do that, or your initial ReactDOM.render() call.

[ljharb]

That seems like a huge flaw in current implementations of HMR, then. It seems like a poor idea to codify a hopefully temporary flaw in an eslint rule, since those tend to be cargo-culted into permanent best practice.

[Ashoat]

Some quick notes here:

1. Nobody uses propTypes anymore, so I don't think that should be a subject of conversation here.
2. As @Alloyed mentioned, this is pretty much the only HMR approach to React in 2022. HMR is pretty much the only way to develop React today. As such, an ESLint rule like this would be hugely valuable to anybody developing React today.

@ljharb, understand that you personally don't want to work on this... I imagine it's frustrating having to enforce a rule that you see as a "temporary flaw". I'm not here to defend this design decision... I don't even have context on why it was made.

But as a React developer, I would love to see an ESLint rule that could help our team avoid breaking HMR. If this needs to be a third-party package I think that's fine. Commenting here to share context, and potentially solicit community interest from others on working on this.

[ljharb]

Lots of people still use PropTypes - the prop-types lib gets 18m downloads a week, so it should always be a subject of conversation anywhere, objectively.

[laSinteZ]

Hey folks, I was looking for a solution and found this rule: https://github.com/ArnaudBarre/eslint-plugin-react-refresh (@Ashoat take a look, it might help you :) Please notice that this rule was created for Vite, but I think it should work still (or you can use it as a starting point for your own implementation)

I am not a big fan of structuring code in a specific way just to make HMR work, but this rule might be a good enough solution for a starter (setting it to warn and having a message "if you want HMR don't do this but that's not required").

Overall it would've been great if react-fast-refresh didn't require such rules to work, but it is what it is and react-fast-refresh is de-facto a standard in a lot of current starters/boilerplates/meta-frameworks (Next.js, Gatsby, CRAv4+, Vite, you name them).

[hichemfantar]

Edit: moved to this issue
I get this error when calling React.forwardRef on my component.

function CardContainer(props,ref) {
	return <div ref={ref}>Hello world!</div>;
}

export default forwardRef(CardContainer);