Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unexported (non-component) constant triggers warning #75

Closed
githorse opened this issue Jan 11, 2025 · 6 comments
Closed

unexported (non-component) constant triggers warning #75

githorse opened this issue Jan 11, 2025 · 6 comments

Comments

@githorse
Copy link

I have a file like this:

const FOO = 42

export function getFoo() {
  return FOO
}

This triggers the Fast refresh only works when a file only exports components warning. Lowercasing fixes the problem, or putting an underscore inside the variable name:

const foo = 42     // ok
const FOO_BAR = 42 // ok
const FOO = 42     // bad
const Foo = 42     // bad

In any case I'm not exporting FOO ... should this trigger the warning?

@ArnaudBarre
Copy link
Owner

The rule is enaled only in JSX/TSX files, so not exporting a function that matches React component naming ([A-Z][A-Za-z]+) there and exporting a constant that matches React component naming is not something expected. You should either:

  • correctly name your React components
  • not using TSX if your fille don't need too

@githorse
Copy link
Author

githorse commented Jan 11, 2025

In my actual case, the file has no actual React component in it, but the getFoo() function actually does contain tsx, so I need the tsx extension. Basically, it's used to derive bits of layout for use in some other React component. Here's a silly but slightly more realistic example:

import React, {ReactNode} from 'react'

const FOO = 42

export function getFoo(bar: boolean): ReactNode {
  if (bar) {
    return String(FOO)
  } else {
    return (
      <div style={{fontWeight: 'bold'}}>
        {String(FOO)}
      </div>
    )
  }
}

I can easily shush the warning by just changing FOO to foo, but it's kind of weird that I need to do that, and I'm left wondering if this file is actually going to break HMR or not. Any advice?

@ArnaudBarre
Copy link
Owner

Why can't getFoo be a Foo/DisplayFoo component?

That's true that this file won't break HMR, because it can't self self update in the first place, this pattern is kind of unsupported but could be without too much work I think, let me check

@ArnaudBarre
Copy link
Owner

The code for local variables was not using the same checks as for exported variables. I'll try to fix the type issue reported by the TS team and published a version

@githorse
Copy link
Author

Yeah ... in this case you're right, that should just be a component. My real code has a lot more going on but boils down to the same issue here. I don't think it's worth getting into the details, but my real getFoo() takes some configuration and outputs an array of column definitions for the Material React Table datagrid. Those column definitions contain a mixture of Javascript objects and JSX; for instance, they have stuff in there like

{
  size: size,
  header: header,
  Edit: ({cell, row}) => {
    return <Checkbox {...} />
  },
  ...
}

So, definitely not a component.

But I end up doing this a lot, actually, writing helper methods that contain JSX. I've never had any issues with it but also haven't stopped to think about how that might impact HMR.

@ArnaudBarre
Copy link
Owner

ArnaudBarre commented Jan 11, 2025

In that case the impact on HMR is the same as any util function, when updating this file it will refresh all the components that imports it. If you want a better understanding, you can look at my talk at Vite conf 2023: https://www.youtube.com/watch?v=woVquKjLs1M

The version was published btw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants