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

Add an option to not warn on const export #8

Closed
JiangWeixian opened this issue Apr 12, 2023 · 2 comments
Closed

Add an option to not warn on const export #8

JiangWeixian opened this issue Apr 12, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@JiangWeixian
Copy link

Thanks for you job! I test on vite based project, when I create a button.tsx file, contains:

import React from 'react'

export const CONSTANT = 12
export const Foo = () => {
  window.document.title = '6'
}

export default function Button() {
  const [count, setCount] = React.useState(0)
  return <div onClick={() => setCount(1)}>{count}1</div>
}

When I edit this file, it's safety hmr update. Do I miss something?

@ArnaudBarre
Copy link
Owner

ArnaudBarre commented Apr 12, 2023

So yeah this file is not respecting the rules of Fast refresh:

  • CONSTANT is not a react component, but will be correctly handle by the Vite plugin because this is easy to diff between files updates. In that exceptional cases, you can ignore the lint rule if the constant is not edited.
  • Foo is lying the eslint plugin (and probably the runtime check if count is kept) because this has the format of a component (function that starts with an uppercase) but the code shows that it's not (technically in recent React version not retuning is allowed, but the render is doing a side effect wihtout useEffect so this clearly not a component). You should rename this function to foo and move it to another file if you want proper updates. If you only rename but don't move, the Vite plugin will invalidate the file on every updates, which will cause the count state to be lost. A way to see that the HMR is not working properly because of this "lie" is to update 6 to something else and rerun the function in another file, this will still uses the old value

I hope it's clear!

Edit: For the constant export, this could be allowed by the plugin via an option though (I don't want to make it a default because I don't think other integrations handle this specific case)

@JiangWeixian
Copy link
Author

JiangWeixian commented Apr 13, 2023

Kapture.2023-04-13.at.10.44.20.mp4

You are correct, I update vite to @4, found when I update foo inside button.tsx, vite will invalidate this file(in @3 it logs hmr update). However, button state looks like kept.

@ArnaudBarre ArnaudBarre changed the title export const foo looks like hmr safety Add an option to not warn on const export Apr 13, 2023
@ArnaudBarre ArnaudBarre added the enhancement New feature or request label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants