ArnaudBarre / eslint-plugin-react-refresh

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

Check for default export naming to be PascalCase #14

Closed jameshulse closed 1 year ago

jameshulse commented 1 year ago

Hello,

When we are using Vite with react-refresh, hot-reloading stops working if we so much as export a component without pascal case naming. If I'm not wrong, this package seems like the best place to enforce a rule to always export from *.jsx files with the correct naming style.

Example without hot reloading:

const myComponent  = () => { ... }

export default myComponent; // Camel case export _not_ pascal case

Working example with hot reloading:

const MyComponent = () => { ... }

export default MyComponent; // Not the pascal-casing

We are able to replicate the issue consistently in our project, but haven't found an ESLint rule that will catch this mistake. Is this already possible? Or would it be a simple enough fix to include it?

Thanks

ArnaudBarre commented 1 year ago

Hi,

I will mark this as out of scope of this plugin for now because I think this convention is well respected in the ecosystem in general and this is not trivial to add to this plugin.

You can create a rule that look for ArrowFunction as top level node of programs from .jsx files, and warn if the return part is a JSXContainer (not sure how this last part is easy I've never done it)

Note: technically this is allowed to have function that takes multiple params and return JSX that are not component (but rarely advised)

jameshulse commented 1 year ago

Okay fair enough.

I was wondering if the logic would simply match what Vite/react-refresh(?) is looking for: "something exported that isn't pascal case".

So you wouldn't actually check the return type of the function etc - you can assume because it is .jsx that it is a component, but warn on the naming only.

Anyway, if it doesn't make sense for this plugin then that's okay. Thanks.

ArnaudBarre commented 1 year ago

Personally it happens for me to have file with some JSX blocks without any components (for example sharing select options between components and one of the field is a callback that return custom JSX rendering)

If you want to make a lint rule that always forbid non pascal case exports in JSX files, starting from this rule and trimming all the unrelated part if probably a good way to get it done

jameshulse commented 1 year ago

Personally it happens for me to have file with some JSX blocks without any components (for example sharing select options between components and one of the field is a callback that return custom JSX rendering)

Fair point. But does Vite hot-reloading work in those cases? It felt like it was sensitive to naming, so would break if that's the case?

ArnaudBarre commented 1 year ago

The React plugins only try to HMR files that contains Refresh code: https://github.com/vitejs/vite-plugin-react-swc/blob/08b41e98f5526d8bd8203c90c79de22b815ab15a/src/index.ts#L113

So normally in that case Babel/SWC should not found anything that could be a React function, so not injecting any React refresh boilerplate and the file will behave like any non TSX file (ie bubble HMR to all the component that imports it)