ArnaudBarre / eslint-plugin-react-refresh

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

Support for constant objects? #18

Closed schaschko closed 1 year ago

schaschko commented 1 year ago

Regarding

{
  "react-refresh/only-export-components": [
    "warn",
    { "allowConstantExport": true }
  ]
}

Is there a way to also support objects? E.g.

export const documentProps = {
  // This title and description will override the defaults
  title: 'About SpaceX',
  description: 'Our mission is to explore the galaxy.'
}
ArnaudBarre commented 1 year ago

I though about it during the implementation but this is very complex to handle if the objet contains reference to other variables. In the case of constant object like this, this should be possible, I will see what I can do (I need to work on upcoming Vite 4.4, so in a few weeks)

ArnaudBarre commented 1 year ago

So I took a look at this and actually this would be very difficult to do because to be safe the object needs to never be updated from inside the module like this:

export const documentProps = {
  // This title and description will override the defaults
  title: 'About SpaceX',
  description: 'Our mission is to explore the galaxy.'
}

documentProps.title = "About NASA"

In that case the importers will still get the reference to the old export which will not received the modifications.

While this is feasible with static analysis, it's complex and this will need to also be done when transforming the React components. The second part is too independant of the plugin and complex if using SWC, so for now I'm on the side of making this pattern incompatible with FastRefresh.

If you have only two properties to export, is it feasible to have two string export? Is this documentProps coming from a framework?

Other idea that can be explored:

schaschko commented 1 year ago

I use it for e.g. exporting meta data per page. In this case it would be feasible, and also possible to refactor the "problem" away.

Regarding the other ideas: if you think about tackling the issue, my feeling would be that it should work "natively".

Thanks for looking into it.