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

Replace eslint/prettier with biome #10

Merged
merged 36 commits into from
Aug 2, 2024
Merged

Replace eslint/prettier with biome #10

merged 36 commits into from
Aug 2, 2024

Conversation

sverhoeven
Copy link
Collaborator

@sverhoeven sverhoeven commented Aug 1, 2024

Followed instructions at https://turbo.build/repo/docs/guides/tools/biome

Also

  • drops the packages/config-typescript/ in favor complete tsconfig.json next to each package.json.
  • adds pnpm typecheck command
  • also includes Prep publish package #9 PR

TODO

  • check unsafe fixes where save
  • fix/ignore errors
  • run workflow once on a push

@sverhoeven sverhoeven changed the base branch from main to prep-publish-package August 2, 2024 07:53
The fix is by default unsafe, but the changes it made looked fine so I marked the fix as safe
@sverhoeven sverhoeven changed the base branch from prep-publish-package to main August 2, 2024 08:11
@sverhoeven sverhoeven marked this pull request as ready for review August 2, 2024 08:16
@sverhoeven sverhoeven requested a review from Peter9192 August 2, 2024 08:16
@Peter9192 Peter9192 mentioned this pull request Aug 2, 2024
Comment on lines 2 to 3
"editor.codeActionsOnSave":{
"source.organizeImports.biome": "explicit"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of adding an auto-sort imports on save. I think this is handy, but feel free to disagree :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I noticed that biome respects grouped imports, i.e. there is no sorting off import statements separated by empty lines

Copy link
Member

@Peter9192 Peter9192 Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also imports from the same module that are on different lines are not sorted or combined. But once you combine them on one line, they will be sorted alphabetically 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool thanks, the organize sort is not part of lint or format commands so its nice the editor can do it.

Copy link
Member

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to have some decent autoformatting! Also great to get rid of these shared configuration packages.

"turbo": "^2.0.6"
},
"packageManager": "[email protected]",
"name": "with-vite"
"name": "class-web"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

packages/class/jsr.json Outdated Show resolved Hide resolved
"test": "jest",
"test:watch": "jext --watch"
"test:watch": "jext --watch",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"test:watch": "jext --watch",
"test:watch": "jest --watch",

but will be changed in #11

turbo.json Outdated
Comment on lines 19 to 20
"//#format-and-lint": {},
"//#format-and-lint:fix": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's with this weird syntax //#? I see it in https://turbo.build/repo/docs/guides/tools/biome#create-a-root-task but I'm not sure why it is like that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so that pnpm exec turbo format-and-lint works, but on my machine

  • 2.7s for pnpm exec turbo format-and-lint (cached)
  • 0.3s for time pnpm format-and-lint

So I will remove it as running biome directly is preferred.

@Peter9192
Copy link
Member

I did notice that the --reporter github options doesn't render nicely in the CI for my breaking commit 639e349

@sverhoeven
Copy link
Collaborator Author

I did notice that the --reporter github options doesn't render nicely in the CI for my breaking commit 639e349

I switched away from biome action. See https://github.com/classmodel/class-web/actions/runs/10214528625 for example error report

Running biome directly is much quicker (2.7s vs 0.3s)
@sverhoeven sverhoeven merged commit b0e3011 into main Aug 2, 2024
2 checks passed
@sverhoeven sverhoeven deleted the biome branch August 2, 2024 11:13
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.

2 participants