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

Fast Refresh not working with automatic runtime and file extension without "x" #83

Closed
7 tasks done
cknitt opened this issue Jan 10, 2023 · 4 comments · Fixed by #122
Closed
7 tasks done

Fast Refresh not working with automatic runtime and file extension without "x" #83

cknitt opened this issue Jan 10, 2023 · 4 comments · Fixed by #122
Labels
enhancement New feature or request

Comments

@cknitt
Copy link

cknitt commented Jan 10, 2023

Describe the bug

Currently, fast refresh is working for a file only if either

  • the file extension ends with an "x" (e.g. "jsx", "tsx") or
  • the code in the file imports React in some way

Now when using a compile-to-JS language like ReScript with the new JSX runtime enabled, one can get .js or .mjs files with imports from "react/jsx-runtime" or "react/jsx-dev-runtime", but no React import, and fast refresh will not work for these files.

Reproduction

https://github.com/cknitt/vite-plugin-react-fast-refresh-issue

Steps to reproduce

  1. Install dependencies and start vite:
npm i
npm run dev
  1. Increment counter
  2. Change the text in TestComponent.bs.js (alternatively change it in TestComponent.res and run npm run res:build to recompile). Observe that the counter value is reset to 0.

Workaround

  1. Run npx patch-package to apply the patch from patches/@vitejs+plugin-react+3.0.1.patch.
  2. Restart vite and repeat the above test steps.
  3. Observe that TestComponent now refreshs correctly and the counter value remains unchanged.

System Info

System:
    OS: macOS 13.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 893.44 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.1 - ~/Library/Caches/fnm_multishells/60384_1673351682301/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.15.0 - ~/Library/Caches/fnm_multishells/60384_1673351682301/bin/npm
    Watchman: 2023.01.02.00 - /opt/homebrew/bin/watchman
  Browsers:
    Brave Browser: 108.1.46.153
    Firefox: 107.0.1
    Safari: 16.2
  npmPackages:
    @vitejs/plugin-react: ^3.0.1 => 3.0.1
    vite: ^4.0.4 => 4.0.4

Used Package Manager

npm

Logs

No response

Validations

@ArnaudBarre
Copy link
Member

ArnaudBarre commented Jan 10, 2023

Thanks for the repro. I will take this into account for the API of the v4

@cknitt
Copy link
Author

cknitt commented Jan 10, 2023

@ArnaudBarre Thanks! 🙂

I think that at

const importReactRE = /(?:^|\n)import\s+(?:\*\s+as\s+)?React(?:,|\s+)/
instead of checking for "React" in e.g. import * as React from 'react';, we should check for the package name (one of "react", "react/jsx-runtime", or "react/jsx-dev-runtime"). Right?

@ArnaudBarre
Copy link
Member

This would work for jsx because this is the only use for this subpath, but not for "react", because this can be a file with just React hooks. This condition needs to be adapted based on the runtime type, I will do that

@ArnaudBarre ArnaudBarre added enhancement New feature or request and removed pending triage labels Jan 22, 2023
@netoya
Copy link

netoya commented Feb 27, 2023

I think with can use

/^[A-Z]([^\.])+\.[t|j]sx?/.test(filepath.split("/").pop())

because all components files start with UpperCase

@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants