ArnaudBarre / eslint-plugin-react-refresh

Validate that your components can safely be updated with Fast Refresh
MIT License
216 stars 14 forks source link

Warning triggered for component using forwardRef #13

Closed hichemfantar closed 1 year ago

hichemfantar commented 1 year ago

v0.41 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);
ArnaudBarre commented 1 year ago

Yep that's normal because you export an anonymous component which is not recommended because this will not be available in devtools and stack traces.

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

export const CardContainer = forwardRef(InnerCardContainer);
hichemfantar commented 1 year ago

Understandable, thanks for the clarification.

hichemfantar commented 1 year ago

One more thing @ArnaudBarre , you said the component will not be available in devtools and stack traces yet the component appears normally in the devtools and in the profiler. examples below: image image

ArnaudBarre commented 1 year ago

Interesting, I never really tested this fact from the React team because for me there is a good reason to name you component and ban default export: https://jamie-wong.com/2013/07/12/grep-test/

hichemfantar commented 1 year ago

Thanks for the article, that was a good read. I ended up using this syntax to avoid renaming the function.

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

export default CardContainer;
ArnaudBarre commented 1 year ago

That could be a challenging refactor, but I advice you to move to export const CardContainer. On top of being simpler to write, it's more align with what I do in other files where I can export a bunch of functions, contants and so on. The default export add a barrier to export multiple React component in the same file, which I used often when there are 2-3 small components related to each other. (And this avoid the component to be imported in another file with a typo/rename and not being greppable (even if IDE & TS will found it, sometimes that's nice to be able to have confidence in the result of a global search))

hichemfantar commented 1 year ago

Named exports is definitely more convenient and easier to follow. I'm currently slowly refactoring default exports to named exports and I'm def noticing improvements. Thanks for the tips, I'm slowly incorporating them into my current project.