Skip to content

chore: replace linter #202

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

Closed
wants to merge 3 commits into from
Closed

Conversation

dougg0k
Copy link
Contributor

@dougg0k dougg0k commented Jul 24, 2025

Replaced eslint with biomejs.

Eslint only has linting where biome has lint, formatter and sort organizer. A much more complete tool.

Previous discussion here https://github.com/callstackincubator/react-native-node-api/pull/200/files#r2229485568

A bunch of linting and formatting fixes.

Copy link

changeset-bot bot commented Jul 24, 2025

🦋 Changeset detected

Latest commit: 234c927

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
gyp-to-cmake Patch
cmake-rn Patch
ferric-cli Patch
@react-native-node-api/test-app Patch
react-native-node-api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@shirakaba
Copy link
Collaborator

At work, we have been stuck on Biome for the last year and this week finally migrated off it back to Prettier + Eslint. Main reasons were:

  • Biome's React linting rules are less powerful (e.g. it can only tell a ref is stable if that ref is declared in the same scope).
  • There are many cases where you have to manually write Biome ignores, because the tooling does not support inserting them via a keyboard shortcut.
  • It co-exists horribly with Prettier. I always have to turn it on as my default formatter in my VS Code user settings, which prevents me from jumping between Prettier and Biome projects.
  • It can't handle a Windows VM running on macOS. It installs the wrong target triplet for the Biome executable, so you need to provide an explicit absolute path to it, which is only specific to your environment, and so that setting can't ever be saved into source-control as it'd affect your teammates.
  • The type-aware linting rules are very early-stage.
  • Prettier is now actually getting faster, with oxc and the new experimental CLI.

The PR itself looks perfectly correct (though I'd encourage configuring the trailing comma settings to reduce the diff), I just have had a bad experience with Biome and if the hard work of setting up the repo for Eslint has already been done, I don't see much to gain from swapping the linter.

@dougg0k
Copy link
Contributor Author

dougg0k commented Jul 25, 2025

It was lacking formatter and sort organizaer (plus).

As mentioned in the other PR, biome found more lint issues and fixed them, where the previous eslint setup didnt even find them with a bunch of packages. So, so far, it's a upgrade.

@dougg0k
Copy link
Contributor Author

dougg0k commented Jul 25, 2025

Here I stopped using prettier, even in vscode, too clunky.

Formatter in biome are just better and faster.

@kraenhansen
Copy link
Collaborator

@dougg0k it will likely hang around for a bit as I'd like to circle this suggestion with other stakeholders of the project too and spend some time with the branch myself. I'm mostly offline for about another week of vacationing 🏝️ and it likely won't merge before I get back, if at all.

@dougg0k dougg0k changed the title chore: replaced linter chore: replace linter Jul 25, 2025
@dougg0k
Copy link
Contributor Author

dougg0k commented Jul 25, 2025

Project lacks a formatter currently, ideally this would be merged, if someone has better impl, they could change later.

@shirakaba @kraenhansen

It's just a tool, this would help me not have to save without format or sometimes have to format with spaces myself.

8f79a56

@kraenhansen
Copy link
Collaborator

I agree we should add a formatter and I opened this PR ~ 2 months ago that I'd like for you to review: #115. It was stalled because we had many open PRs and merging it would cause a lot of conflicts. We still have a bunch, but it might be a good time to revive it.

I surfaced this PR with the rest of the project stakeholders and we're happy with an eslint + prettier setup for now. I agree we do need to tune the rules and plugins further and would love suggestions on that - ideally we'd want to stick pretty close to recommended settings for now.

@dougg0k
Copy link
Contributor Author

dougg0k commented Jul 25, 2025

Maybe merge that then.

@dougg0k dougg0k closed this Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants