biomejs / biome

A toolchain for web projects, aimed to provide functionalities to maintain them. Biome offers formatter and linter, usable via CLI and LSP.
https://biomejs.dev
Apache License 2.0
14.07k stars 428 forks source link

📎 Implement `eslint-plugin-react-refresh` #3560

Open GunseiKPaseri opened 1 month ago

GunseiKPaseri commented 1 month ago

Description

from https://github.com/biomejs/biome/discussions/3#discussioncomment-9177559

react-refresh

react-refresh is an official package of react that provides bundler with a Fast Refresh feature that allows real-time updates of running React apps. In addition to react-native, next.js, @vitejs/plugin-react and @parcel/runtime-react-refresh are often used behind the scenes in development environments. It is a so-called hot reload, but it is characterized by the fact that only edited components can be updated minute by minute while maintaining the state. For more detailed information, please refer to the documentation of each bundler.

eslint-plugin-react-refresh

eslint-plugin-react-refresh is a plugin also linked from @vitejs/plugin-react that defines rules to make react-refresh function properly.

Rule react-refresh/only-export-components is too large.

Already implemented

I plan to implement it.

The following are not included in the current lint rules, but they are desired for future implementation.

more info: https://github.com/biomejs/biome/discussions/2548

GunseiKPaseri commented 1 month ago
arendjr commented 1 month ago

For this extension, we want to apply it only to .jsx and .tsx files

Note that while TypeScript is strict about requiring a .tsx extension for files with JSX syntax, such a requirement doesn't exist for JavaScript. A .js file with JSX syntax can be totally fine depending on one's bundler configuration.

Conaclos commented 1 month ago

The main lint rule also outputs errors equivalent to noReExportAll. What would be an appropriate way to guide the enabling of this during migration?

Unfortunately Biome migration tool doesn't support a source to be associated to several Biome rules. Thus, for now, it is not possible to do that.

For this extension, we want to apply it only to .jsx and .tsx files. Since it seems that this cannot be achieved by specifying language: "jsx", is it appropriate to implement this check within the run function?

Could you explain the reasoning behind this choice? If it is really necessary, you can retrieve the file path (something like ctx.file_path()) and check the extension with the standard library.

There is a function that we want to share with other nursery rules (noYodaExpression). Where would be the appropriate place to extract this function?

If the function is quite specific, you can just make it public in one of the rule (pub(crate)) and import it in the other. If the function is quite generic and seems useful in a general way, then you can place it in the biome_js_syntax crate. We have a bunch of files ending with _ext where it could be placed.

In addition, since the Pascal case is used to determine whether a component is a component or not, we plan to implement functions related to the constraints around it.

biome_string_case::Case::identify should be helpful in that regard :)

GunseiKPaseri commented 1 month ago

Unfortunately Biome migration tool doesn't support a source to be associated to several Biome rules. Thus, for now, it is not possible to do that.

mmm...

Could you explain the reasoning behind this choice? If it is really necessary, you can retrieve the file path (something like ctx.file_path()) and check the extension with the standard library.

In detecting React components, only the name is used, which can lead to false positives. The original lint rule was implemented to avoid this issue.

Conaclos commented 1 month ago

Unfortunately Biome migration tool doesn't support a source to be associated to several Biome rules. Thus, for now, it is not possible to do that.

Although, it could be fairly easy to change how we generate migrate and website pages to allow this.