UN-3185 [MISC] : replace ESLint and Prettier with Biome for frontend #1771
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… formatting - Add @biomejs/biome v2.3.13 as dev dependency - Create biome.json with migrated ESLint rules and formatter config - Update package.json scripts (lint, format, check) to use Biome - Remove ESLint and Prettier dependencies and .eslintrc.json - Add Biome pre-commit hook to .pre-commit-config.yaml - Fix undeclared sessionDetails variable in HeaderTitle.jsx - Fix Error() and Array() to use new keyword - Fix prismjs import order in CombinedOutput.jsx - Reformat all frontend source files with Biome
Contributor
|
Important Review skippedToo many files! This PR contains 236 files, which is 86 over the limit of 150. You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jaseemjaskp
requested review from
athul-rs,
hari-kuriakose and
vishnuszipstack
- Add biome-check to ci.skip list since pre-commit.ci lacks reliable npm registry access for language:system hooks - Add --files-ignore-unknown=true and --no-errors-on-unmatched flags to the biome-check hook entry for robustness
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
8 tasks
jaseemjaskp
changed the title
| ci: | ||
| skip: | ||
| - hadolint-docker # Fails in pre-commit CI | ||
| - biome-check # Uses local npm/npx, not available in pre-commit CI |
Contributor
There was a problem hiding this comment.
@jaseemjaskp does this mean that we don't plan to check for linting in CI? Should we use biome's hook itself?
Also our experience from having a local mypy hook was not that great, it was inconsistent. I hope we don't run into such pitfalls and the lint command and pre-commit hook all reliably use the same config
Contributor
Author
There was a problem hiding this comment.
@chandrasekharan-zipstack the pre-commit CI failed while running frontend-related hooks that use npm/npx. This seems to be a limitation of the official pre-commit setup.
We’ll need to run these checks as a separate GitHub Actions workflow instead.
| hooks: | ||
| - id: biome-check | ||
| name: biome check | ||
| entry: bash -c 'cd frontend && npx @biomejs/biome check --write --files-ignore-unknown=true --no-errors-on-unmatched "${@#frontend/}"' -- |
Contributor
There was a problem hiding this comment.
@jaseemjaskp to keep it consistent and maintain a single config for all linting, we should instead have
bun run lint:fix here. All args and config that's needed for linting can reside in package.json alone.
That way, while using the pre-commit hook and the command directly - we'd always get the same result. In the current implementation we are setting ourselves up for some drift in config eventually
Contributor
Author
There was a problem hiding this comment.
@chandrasekharan-zipstack Biome runs using its own config file, so it’s independent of package.json. Because of that, its behavior stays consistent whether it’s run via package.json scripts or through a pre-commit hook.
jaseemjaskp
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Replace ESLint + Prettier with Biome for frontend linting and formatting.
Changes
@biomejs/biomev2.3.13 as dev dependencybiome.jsonwith rules migrated from.eslintrc.json(68 rules auto-migrated)package.jsonscripts (lint,format,check, etc.) to use Biome commandseslint,eslint-config-google,eslint-config-prettier,eslint-plugin-prettier,eslint-plugin-react,prettier,prettier-quick.eslintrc.json.pre-commit-config.yamlBug Fixes (discovered during migration)
sessionDetailsvariable inHeaderTitle.jsx(added missinguseSessionStoreimport)Error()→new Error()inuseSessionValid.jsArray()→new Array()inDocumentThumbnail.jsxprismjsimport order inCombinedOutput.jsx(core library must load before addons)Why Biome?
biome.json) instead of multiple configsorganizeImportsNotes
warnfor pre-existing code issues:useBlockStatements(319),noUnusedVariables(108),noEmptyBlockStatements(17) — these can be gradually fixed in follow-up PRs