-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix: don't count class declarations as react fast refresh boundry (fixes #3675) #8887
fix: don't count class declarations as react fast refresh boundry (fixes #3675) #8887
Conversation
✅ Deploy Preview for vite-docs-main canceled.
|
Personally, I wouldn't consider this a minor bug. Why? The beginning of the docs tell us:
If HMR is broken so badly that I have to manually refresh the page each time I update a component, Vite can no longer be considered "quick" for development, and hence the entire point of using Vite is defeated and I might as well go back to using webpack. |
I was not able to reproduce. Editing class components caused a full reload properly for me. I tested the components below. import React from 'react'
function connect() {
return function(component) {
return component
}
}
class ClassComponent extends React.Component {
render() {
return <p>Hello from class component.</p>
}
}
export const WrappedClassComponent = connect()(ClassComponent) import React from 'react'
class ClassComponent extends React.Component {
render() {
return <p>Hello from class component.</p>
}
}
export default ClassComponent import React from 'react'
export class ClassComponent extends React.Component {
render() {
return <p>Hello from class component.</p>
}
} |
Thanks for your reply. I created a minimal reproduction of the bug here: https://github.com/DouglasDev/vite-hmr-bug to reproduce:
Note: I think having both a functional and class component in the same file has something to do with the bug. |
I think so. When a functional component exists (it does not need to be an exported one), vite/packages/plugin-react/src/index.ts Lines 355 to 358 in e6f3b02
|
@patak-dev @sapphi-red I tried updating my packages to the latest version and I'm still seeing this bug occur. Did either of you test that my fix worked? I wasn't able to figure out how to test it myself (explained in the "Additional context"). I can make a second attempt at a fix if someone can walk me through how to run the |
I did test this. It seems it works for named exports, but not for default export. export function App() {
return <div className="App"></div>
}
export class App2 extends Component {
constructor(props){
super(props)
}
render(){
return <div className="App"></div>
}
} |
@sapphi-red If you can give me some guidance as to how to test the react plugin in development, I can try to write a fix. |
I'm not sure why this is not working. Did you run |
Description
This addresses vitejs/vite-plugin-react#8 especially: vitejs/vite-plugin-react#8
If an export in a Vite React project is a React class component, editing the contents of the components and then saving the file will not cause any updates in the browser during development (neither a fast refresh nor a full refresh). I fixed this by checking for class components in the
isRefreshBoundary
and returning false if one is found.Additional context
I'm pretty sure this will work but I wasn't able to test locally. I tried adding the follow to my package.json in a test React project:
however the changes in the
plugin-react
didn't seem to be reflected in my project.If we could merge this into v2 of vite as well, that would be appreciated since this bug is creating a worse dev experience for many projects on v2.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes vitejs/vite#123
).