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

feat: biome lint frontend #4903

Merged
merged 6 commits into from
Oct 2, 2023
Merged

feat: biome lint frontend #4903

merged 6 commits into from
Oct 2, 2023

Conversation

nunogois
Copy link
Member

@nunogois nunogois commented Oct 2, 2023

Follows up on #4853 to add Biome to the frontend as well.

image

Added a few biome-ignore to speed up the process but we may want to check and fix them in the future.

@vercel
Copy link

vercel bot commented Oct 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 2, 2023 11:02am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Oct 2, 2023 11:02am

Copy link
Contributor

@chriswk chriswk left a comment

Choose a reason for hiding this comment

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

I love it. Does it work?
Some thoughts:

  • No changes to package.json in frontend,
    doesn't that mean that applying yarn lint in the frontend folder will still call eslint?
  • I see we're applying backend's preference for single quotes to frontend as well. Is React JSX/TSX fine with that?

@nunogois
Copy link
Member Author

nunogois commented Oct 2, 2023

I love it. Does it work? Some thoughts:

  • No changes to package.json in frontend,
    doesn't that mean that applying yarn lint in the frontend folder will still call eslint?
  • I see we're applying backend's preference for single quotes to frontend as well. Is React JSX/TSX fine with that?

Thank you for those good questions @chriswk

Does it work?

Seems to work fine now and I finally fixed the failing tests. Mind testing locally on your machine just to be sure?

No changes to package.json in frontend,
doesn't that mean that applying yarn lint in the frontend folder will still call eslint?

You're right. This was addressed in f0f33c1

It's not a big deal since it is super fast, but it would be great if we could drop these commands in frontend altogether and rely only on the top level ones, since those cover everything.

It's also weird that they act differently in backend and frontend.

  • Backend: yarn lint will check your files and yarn lint:fix will attempt to fix them. This check covers both linting and formatting for both frontend and backend;
  • Frontend: yarn lint will attempt to lint your frontend files and yarn lint:check will lint-check your frontend files. yarn fmt will attempt to format your frontend files and yarn fmt:check will format-check your frontend files.

I see we're applying backend's preference for single quotes to frontend as well. Is React JSX/TSX fine with that?

AFAIK it's totally fine.

@chriswk
Copy link
Contributor

chriswk commented Oct 2, 2023

Ran tests locally, everything seems to be green. Don't worry about Codacy at the moment. I think we either need to have a close look at the config or just drop it.

@nunogois nunogois merged commit 4167a60 into main Oct 2, 2023
14 of 16 checks passed
@nunogois nunogois deleted the feat-biome-lint-frontend branch October 2, 2023 12:25
nunogois added a commit that referenced this pull request Oct 2, 2023
Tiny refactor that bubbles promises instead of using `return await`.
Should be more consistent with the rest of the changes in
#4903
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants