Add Biome.js for local linting #5587

sidharthv96 · 2024-06-21T09:22:27Z

📑 Summary

Adds Biome.js for local linting and formatting.

📏 Design Decisions

ESLint has been causing memory issues, and the pre-commit hook is really slow.
Replacing pre-commit hook with biome enhances the productivity.

Pros:

Fast & light (won't take minutes for every commit)
Has import sorting, without removing unused imports.
Cons:
Does not support all rules (No type aware rules, so we will still need eslint in CI)
Not all errors will be caught in pre-commit, and CI will fail. But it's better than waiting minutes for each commit, or eslint crashing everytime.

TODO:

Remove rules in eslint that are supported in biome.

📋 Tasks

Make sure you

📖 have read the contribution guidelines
💻 have added necessary unit/e2e tests.
📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
🔖 targeted develop branch

netlify · 2024-06-21T09:22:48Z

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	`94fa235`
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/66793a980bbda10008486c0d
😎 Deploy Preview	https://deploy-preview-5587--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

We should replace this with Biome as well, but it's a bit complicated.

argos-ci · 2024-06-21T09:54:00Z

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	Jun 24, 2024, 9:34 AM

codecov · 2024-06-21T10:56:45Z

Codecov Report

Attention: Patch coverage is 2.32558% with 168 lines in your changes missing coverage. Please review.

Project coverage is 5.73%. Comparing base (ce3b0af) to head (94fa235).

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #5587      +/-   ##
==========================================
- Coverage     5.73%   5.73%   -0.01%     
==========================================
  Files          278     280       +2     
  Lines        42019   42038      +19     
  Branches       516     492      -24     
==========================================
  Hits          2409    2409              
- Misses       39610   39629      +19

Flag	Coverage Δ
unit	`5.73% <2.32%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
.esbuild/build.ts	`0.00% <0.00%> (ø)`
.esbuild/jisonPlugin.ts	`0.00% <0.00%> (ø)`
.esbuild/util.ts	`0.00% <0.00%> (ø)`
.eslintrc.cjs	`0.00% <0.00%> (ø)`
.lintstagedrc.mjs	`0.00% <0.00%> (ø)`
.vite/jsonSchemaPlugin.ts	`0.00% <0.00%> (ø)`
...ckages/mermaid/src/diagrams/gantt/ganttRenderer.js	`0.00% <0.00%> (ø)`
...ges/mermaid/src/diagrams/state/stateRenderer-v2.js	`0.00% <0.00%> (ø)`
packages/mermaid/src/diagrams/timeline/styles.js	`0.00% <0.00%> (ø)`
packages/mermaid/src/docs/.vitepress/config.ts	`0.00% <0.00%> (ø)`
... and 26 more

... and 1 file with indirect coverage changes

aloisklink

I think making two separate PRs for changing linting and changing prettier/formatting would be easier to understand.

But, I'm also not a big fan of having Biome linting overlapping with ESLint's linting, e.g. us having to both add biome-ignore lint/suspicious/noFallthroughSwitchClause and eslint-disable no-fallthrough.

Would it be better to instead use Oxlint for linting? It's literally designed for our use-case https://oxc.rs/docs/guide/usage/linter.html:

At the current stage, oxlint is not intended to fully replace ESLint; it serves as an enhancement when ESLint's slowness becomes a bottleneck in your workflow.

We recommend running oxlint before ESLint in your lint-staged or CI setup for a quicker feedback loop, considering it only takes a few seconds to run on large codebases.

There's an eslint-plugin-oxlint that automatically disables all ESLint rules that oxlint handles for you.

Biome still seems pretty nice for styling/formatting, though, so I think it can probably replace prettier. And I love the import sorting ability too.

Co-authored-by: Alois Klink <[email protected]>

* develop: chore: update browsers list chore(deps): update all patch dependencies docs: Added demo diagram of bidirectional arrows for sequence diagrams fix(deps): update all patch dependencies

sidharthv96 · 2024-06-24T09:27:16Z

Thanks for the eslint-plugin-oxlint suggestion, @aloisklink. I've added that.
I don't think we should put extra effort to split this into two separate PRs, as the formatting changes are very minimal, and biome replaces prettier pretty good.
Biome and oxc are both viable options, and we can switch to oxc in case we have any hiccups with biome, or we have a strong benefit with oxc.

aloisklink

The linting aspect seems okay to me (although I'd rather keep pnpm lint and pnpm lint:fix doing everything, to prevent confusion with CI failing).

For the formatting conversion, I still think there's some stuff to do.

Are we're still keeping prettier for markdown and HTML files, since biome doesn't support them yet. Can we update the lint commands for this?

I think we also need to describe the changes in the PR description before merging it, e.g. summarizing what has changed, and what the new workflow is for contributors.

Something like:

Add biome as an additional linter, and replace prettier with biome's formatter for JavaScript, TypeScript, and JSON files.

Git pre-commit hooks will now only run biome, to speed up git commits.
ESLint will still be ran in CI and can be manually run with pnpm xxxxxxxxxxx.

Some info about prettier here, depending on what we do with it.

aloisklink · 2024-06-24T12:10:24Z

package.json

+ "lint": "pnpm biome check && pnpm lint:jison",
+ "lint:fix": "pnpm biome check --write",
+ "lint:ci": "pnpm lint && cross-env NODE_OPTIONS=--max_old_space_size=16384 eslint --cache --cache-strategy content .",


I'm not a big fan of making the CI linting different from local linting. It also means people will have to manually fix ESLint issues/CSpell issues.

Having git commit be fast is nice, but if I'm running pnpm run lint, it's usually because I want to check if something is going to pass CI or not.

Would it be better to make a lint:fast or lint:slow script?

Suggested change

"lint": "pnpm biome check && pnpm lint:jison",

"lint:fix": "pnpm biome check --write",

"lint:ci": "pnpm lint && cross-env NODE_OPTIONS=--max_old_space_size=16384 eslint --cache --cache-strategy content .",

"lint:fast": "pnpm biome check && pnpm lint:jison",

"lint:fast:fix": "pnpm biome check --write",

"lint": "pnpm lint:fast && cross-env NODE_OPTIONS=--max_old_space_size=16384 eslint --cache --cache-strategy content .",

"lint:fix": "pnpm lint:fast:fix && cross-env NODE_OPTIONS=--max_old_space_size=8192 eslint --cache --cache-strategy content --fix . && tsx scripts/fixCSpell.ts",

If pnpm lint skips running eslint by default, I'd recommend adding something like: `echo 'Skipping slow eslint-ing, run 'pnpm lint:slow' if you want to test this'.

Whichever option you pick, please update all mentions of lint:fix, e.g. in like:

mermaid/run

Lines 80 to 81 in 4a7489a

$(bold ./$name pnpm -w run lint:fix)

Run prettier and ES lint

and

mermaid/.github/workflows/lint.yml

Lines 43 to 47 in 3a5793f

ERROR_MESSAGE+=' Running `pnpm -w run lint:fix` may fix this issue. '

ERROR_MESSAGE+=" If this error doesn't occur on your local machine,"

ERROR_MESSAGE+=' make sure your packages are up-to-date by running `pnpm install`.'

ERROR_MESSAGE+=' You may also need to delete your prettier cache by running'

ERROR_MESSAGE+=' `rm ./node_modules/.cache/prettier/.prettier-cache`.'

aloisklink · 2024-06-24T12:47:38Z

.lintstagedrc.mjs

@@ -1,10 +1,6 @@
 export default {
 '!(docs/**/*)*.{ts,js,html,md,mts}': [


It sounds like biome doesn't yet support HTML and MD files:

https://biomejs.dev/internals/language-support

aloisklink · 2024-06-24T12:49:14Z

.lintstagedrc.mjs

@@ -1,10 +1,6 @@
 export default {
 '!(docs/**/*)*.{ts,js,html,md,mts}': [
- 'eslint --cache --cache-strategy content --fix',
- // don't cache prettier yet, since we use `prettier-plugin-jsdoc`,


We can probably remove prettier-plugin-jsdoc, if prettier will no longer be used for linting JS code.

sidharthv96 added 8 commits June 21, 2024 14:44

feat: Add biome

2c80d80

chore: Biome auto fixes

08dfdfe

chore: Biome fix for-of loops

3381717

chore: Update lint scripts

22a00a5

chore: Remove unused deps

98f32bf

chore: Add biome renovate config

0a5315c

chore: Enable eslint in CI

3a5793f

chore: Add .eslintignore

a23b891

Fix prettier formatting on generation.

ba34386

We should replace this with Biome as well, but it's a bit complicated.

sidharthv96 added 5 commits June 21, 2024 15:28

chore: Add eslint-disable

d2b42eb

chore: Add eslint-disable

e7577ed

chore: remove comments in json

55a8e4c

chore: Fix shape names

a1badd5

chore: Fix gantt test

89e061a

sidharthv96 and others added 2 commits June 21, 2024 16:40

Merge branch 'develop' into sidv/biome

054ed6c

Format on pre-commit

48303a0

sidharthv96 requested a review from aloisklink June 22, 2024 16:28

aloisklink requested changes Jun 23, 2024

View reviewed changes

sidharthv96 and others added 3 commits June 24, 2024 14:25

chore: Add eslint-config-biome

422b8cd

Co-authored-by: Alois Klink <[email protected]>

chore: Remove redundant eslint comments

8cbe3e0

Merge branch 'develop' into sidv/biome

94fa235

* develop: chore: update browsers list chore(deps): update all patch dependencies docs: Added demo diagram of bidirectional arrows for sequence diagrams fix(deps): update all patch dependencies

aloisklink reviewed Jun 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Biome.js for local linting #5587

Add Biome.js for local linting #5587

sidharthv96 commented Jun 21, 2024 •

edited

Loading

netlify bot commented Jun 21, 2024 •

edited

Loading

argos-ci bot commented Jun 21, 2024 •

edited

Loading

codecov bot commented Jun 21, 2024 •

edited

Loading

aloisklink left a comment

sidharthv96 commented Jun 24, 2024

aloisklink left a comment

aloisklink Jun 24, 2024

aloisklink Jun 24, 2024

aloisklink Jun 24, 2024

- "lint": "pnpm biome check && pnpm lint:jison",
- "lint:fix": "pnpm biome check --write",
- "lint:ci": "pnpm lint && cross-env NODE_OPTIONS=--max_old_space_size=16384 eslint --cache --cache-strategy content .",
+ "lint:fast": "pnpm biome check && pnpm lint:jison",
+ "lint:fast:fix": "pnpm biome check --write",
+ "lint": "pnpm lint:fast && cross-env NODE_OPTIONS=--max_old_space_size=16384 eslint --cache --cache-strategy content .",
+ "lint:fix": "pnpm lint:fast:fix && cross-env NODE_OPTIONS=--max_old_space_size=8192 eslint --cache --cache-strategy content --fix . && tsx scripts/fixCSpell.ts",

	$(bold ./$name pnpm -w run lint:fix)
	Run prettier and ES lint

	ERROR_MESSAGE+=' Running `pnpm -w run lint:fix` may fix this issue. '
	ERROR_MESSAGE+=" If this error doesn't occur on your local machine,"
	ERROR_MESSAGE+=' make sure your packages are up-to-date by running `pnpm install`.'
	ERROR_MESSAGE+=' You may also need to delete your prettier cache by running'
	ERROR_MESSAGE+=' `rm ./node_modules/.cache/prettier/.prettier-cache`.'

		@@ -1,10 +1,6 @@
		export default {
		'!(docs/*/)*.{ts,js,html,md,mts}': [

Add Biome.js for local linting #5587

Are you sure you want to change the base?

Add Biome.js for local linting #5587

Conversation

sidharthv96 commented Jun 21, 2024 • edited Loading

📑 Summary

📏 Design Decisions

📋 Tasks

netlify bot commented Jun 21, 2024 • edited Loading

✅ Deploy Preview for mermaid-js ready!

argos-ci bot commented Jun 21, 2024 • edited Loading

codecov bot commented Jun 21, 2024 • edited Loading

Codecov Report

aloisklink left a comment

Choose a reason for hiding this comment

sidharthv96 commented Jun 24, 2024

aloisklink left a comment

Choose a reason for hiding this comment

aloisklink Jun 24, 2024

Choose a reason for hiding this comment

aloisklink Jun 24, 2024

Choose a reason for hiding this comment

aloisklink Jun 24, 2024

Choose a reason for hiding this comment

sidharthv96 commented Jun 21, 2024 •

edited

Loading

netlify bot commented Jun 21, 2024 •

edited

Loading

argos-ci bot commented Jun 21, 2024 •

edited

Loading

codecov bot commented Jun 21, 2024 •

edited

Loading