Fast Refresh not working with hooks returning JSX/TSX

[mpressmar]

Describe the bug

We are frequently using custom hooks that will return rendering callbacks in which we refer to actual tsx components.

Is this a bug, or is there any alternative for defining component-based hooks which doesn't break HMR?

Reproduction

https://stackblitz.com/edit/vitejs-vite-zzqhfn?file=src%2Fhook.jsx

Steps to reproduce

A simple example:

hook.tsx

import { useCallback } from "react";

export const useExampleHook = () => {
    return useCallback(() => <span>Test</span>, []);
}

If you use this hook in a component, and then modify the hook's source code, the following will be printed in the console:

5:03:45 PM [vite] hmr invalidate /src/hook.tsx Could not Fast Refresh. Learn more at https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-react#consistent-components-exports

System Info

Linux Mint, Firefox, Vite 5.2.11, latest vite react plugin

Used Package Manager

npm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[ArnaudBarre]

This is a duplicate of #289, but thanks for the small repro I'll look into it I didn't though it was that easy to trigger

[dbarabashh]

if i'm not mistaken, plan uses the regular expression defaultIncludeRE = /\.[tj]sx?$/ to identify the files that need to be processed. i think it makes more sense to check for the presence of jsx syntax in the code and implement something like this

if (tsRE.test(id) && !id.endsWith('.tsx')) {
      const hasJsx = jsxContentRE.test(code)
      if (hasJsx) {
        this.warn(
          `File ${id} contains JSX but has a .ts extension. ` +
            `Consider renaming to .tsx or adding "// @jsx" pragma comment.`,
        )
      }
}

now, when using hooks with jsx in .ts files, you'll get a warning that will guide you to properly configure your project. if i understood correctly, this should solve the problem somehow. but i'm not entirely sure how to properly test it considering what we have in the playground. overall, if this is what's needed, i could try to implement it

cc @ArnaudBarre

[ArnaudBarre]

The issue is about having correct JSX configure or about file extensions. The code in the issue should correctly get handled but it's not due to how fast refresh works (I think it's possible to do fast refresh for hooks only, but I never correctly implemented it, and it creates this annoying warning).

[dbarabashh]

then here we need to add a check for the presence of jsx or createElement() as well, right?

vite-plugin-react/packages/plugin-react/src/index.ts

Line 185 in ab12dfa

const isJSX = filepath.endsWith('x')

its just that this check doesnt cover all the cases i think

[ArnaudBarre]

The issue is not detecting is the file is JSX, the issue is correctly detecting if the file should be HMRed.
Removing Sig from const refreshContentRE = /\$Refresh(?:Reg|Sig)\$\(/ should fix the warning, but I want to investigate how fast refresh for hooks only works. to this is this can be lead to cleaner HMR