fix: fix fast-refresh for files that are transformed into jsx

[csr632]

Description

Previous useFastRefresh check is wrong. It causes fast-refresh not working for files with extension such as .md. Checkout the updated playground and the test as a demo.

Additional context

The first commit update the test that fails and demonstrates the issue. The second commit fix the issue and make the test pass.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[ArnaudBarre]

Oh yeah this check is clearly a refactor issue.

What's the use case for having JSX inside .md?

[csr632]

What's the use case for having JSX inside .md?

.md is transformed into a jsx syntax, and is expected to have fast-refresh added by vite-plugin-react (so that editing .md files won't trigger full page reload). But the current useFastRefresh check result is false and vite-plugin-react is not transforming it. (So currently editing .md files is triggering full page reload)

Currently .mdx is working because isJSX(which is filepath.endsWith('x')) is true. But it is not working for.md.

mdx.js support both md format and mdx format: https://mdxjs.com/packages/mdx/#optionsformat

[jprosevear]

Possibly related - I have a project ported over from create-react-app (and which used JS files). HMR will not work on .js files until I do one of the following:

1. Add import React from 'react'; at the top of the file

2. Change the file extension to .jsx

[csr632]

on .js files

Vite emits error if you use jsx syntax in .js file. Why ary you not getting error when doing this? Are you using extra babel rules to do the jsx transform?

Example: https://stackblitz.com/edit/vitejs-vite-cyjzmi?file=src%2FApp.js

Can you reproduce your issue in this demo?

[jprosevear]

    // https://github.com/vitejs/vite/discussions/3448#discussioncomment-749919
    esbuild: {
      loader: 'jsx',
      include: /src\/.*\.jsx?$/,
      // loader: "tsx",
      // include: /src\/.*\.[tj]sx?$/,
      exclude: []
    },
    optimizeDeps: {
      esbuildOptions: {
        plugins: [
          {
            name: 'load-js-files-as-jsx',
            setup(build) {
              build.onLoad({ filter: /src\/.*\.js$/ }, async (args) => ({
                loader: 'jsx',
                contents: await fs.readFile(args.path, 'utf8')
              }));
            }
          }
        ]
      }
    }

[jprosevear]

https://stackblitz.com/edit/vitejs-vite-p5o1nn?file=src%2FApp.js

(Changing something in App.js)

[csr632]

After some digging, I think the issue is similar (incorrect useFastRefresh) but it is not fixed by this PR. I have added a reproduction to the plugin repo: https://github.com/csr632/vite-plugin-react-1/tree/react-js-ext-demo.
So the useFastRefresh judgement is still very fragile even after this PR. It can be improve, but should we improve it for this special case? As the discuession vitejs/vite#3448 (comment) said, using jsx syntax in .js file is not recommended. I would recommend you to migrate .js to .jsx instead: vitejs/vite#3448 (comment)

[jprosevear]

Its not unreasonable to js -> jsx, I was just trying to minimize the number of changes in order to do the initial port. It may also be a common case converting from create-react-app which may help uptake.

[jprosevear]

After some digging, I think the issue is similar (incorrect useFastRefresh) but it is not fixed by this PR.

I tried locally installing the package from a local repo checkout with this change. As you note, didn't work.