Setup linting and formatting with Biome

[iivvaannxx]

Fixes #43 and supersedes #58.

I’ve started incorporating Biome into the repository, so far so good everything works within VSCode. Biome is able to parse all the source code, lint and format it, this includes the files with the following extensions (that I checked): [.js, .ts, .jsx, .tsx, .astro, .json].

I am opening this PR as a draft because I was mid-way fixing the linting errors, I am not at home and Codespaces is the biggest crap I’ve ever tried, horrible DX, constantly crashing and super slow. I will continue at home.

[!NOTE] To speed things up, I’ve used a biome.json config file I had in one of my other projects, so the rules may be a little opinionated to some people, I am open to discuss them if necessary.

As a replacement for @siguici CI workflow to ensure the style is consistent for every contribution, I propose a pre-commit Git hook, which is explained in the Biome docs. I’ve used this approach and it’s pretty comfortable. The docs show how you can either “stop” a commit if it has formatting or linting errors, or how to automatically run the formatter (in case there are no more things to fix) before a commit. I prefer the former to actually acknowledge everything is right before you commit anything, but the latter is also good to me if preferred.

TODO

• [X] Setup Biome
• [X] Fix all linter errors
• [X] Ensure all the code is formatted
• [X] Add pre-commit hook via lefthook
• [X] Update contributing guide to explain about the commit hook and the biome setup.
• [ ] Ensure everything works as expected

@thejackshelton Before merging this I would like you to review and test that everything works as expected, not that I am changing the source code very much or anything, but while fixing the linting errors some rules require to “rewrite” some parts of the code to accomodate these rules. If any unexpected issue arises we can look into it (I will try not to make it happen).

And as a final note, this PR also contains a very subtle change that shouldn’t affect anything but I noticed it while installing Biome. I fixed it because I didn’t think a single PR for it would be of any help. This is moving the changesets dependency from dependencies to devDependencies in the root package.json

[iivvaannxx]

I am not used to making PRs but I just noticed that the two commits of the other PR I recently made at #70 also appear in this PR, not sure if this is going to conflict or something, if it does, I’ll try to fix it from another branch.

I suppose this happened because I created my biome-linting-formatting branch out of the other one and I didn’t notice.

[iivvaannxx]

I think this would be all it's needed, I'm marking the PR ready for review. @thejackshelton I will leave that to you, as I don't know as much of you how the plugin should behave.

I am not a native English speaker, although I tried to explain myself the best I could I probably made some grammar mistakes or typos when updating the contributing guide, if you want to correct anything feel free!

Finally, another subtle change this PR includes is moving the type declarations that were inside the custom-types folder in the src of libs/qwikdev-astro to the env.d.ts of the same folder. Whenever you need to declare custom global types I think that's where they are supposed to go.

[iivvaannxx]

The docs show how you can either “stop” a commit if it has formatting or linting errors, or how to automatically run the formatter (in case there are no more things to fix) before a commit. I prefer the former to actually acknowledge everything is right before you commit anything, but the latter is also good to me if preferred.

One last thing, about what I said, and related to my "warning" in the contributing guide, which is:

It could happen that the hook doesn't trigger if you haven't run lefthook install on your local repo before commiting. Although this command is automatically executed by your package manager after running pnpm install, if it hasn't been executed, you would be bypassing this restriction.

Probably it would be best to enforce the style directly, as I said it's an option, so we can do whatever you find more convenient @thejackshelton.

It can be done by adding --apply to the pre-commit hook, on the executed Biome command, at file lefthook.yaml.

[thejackshelton]

WOW! Awesome job on a detailed and awesome PR so quickly @iivvaannxx! Gonna grab a bite and go over this in the next hour 😄

[iivvaannxx]

Thanks @thejackshelton! Any question or doubt you have I can answer it!

[thejackshelton]

I’ve started incorporating Biome into the repository, so far so good everything works within VSCode. Biome is able to parse all the source code, lint and format it, this includes the files with the following extensions (that I checked): [.js, .ts, .jsx, .tsx, .astro, .json].

Sweet! That's awesome.

I am opening this PR as a draft because I was mid-way fixing the linting errors, I am not at home and Codespaces is the biggest crap I’ve ever tried, horrible DX, constantly crashing and super slow. I will continue at home.

Ah sorry you had to go through that 😓

To speed things up, I’ve used a biome.json config file I had in one of my other projects, so the rules may be a little opinionated to some people, I am open to discuss them if necessary.

Happy to use your best judgement here! If we do run into this case where we want to change a config option, think this is something we should open a github discussion for?

I prefer the former to actually acknowledge everything is right before you commit anything, but the latter is also good to me if preferred.

Pre-commit hook is great! So your preferred approach is it prevents you from committing along with a warning if there are any formatting errors?

Some things I've noticed btw, is doing something like divq (a syntax error) in a tsx file will get biome to shout at us, but it does not do the same in an Astro file 🤔. I guess this would be the partial support that biome mentioned.

type="button" on the button was interesting haha.

[thejackshelton]

I am not used to making PRs but I just noticed that the two commits of the other PR I recently made at #70 also appear in this PR, not sure if this is going to conflict or something, if it does, I’ll try to fix it from another branch.

I suppose this happened because I created my biome-linting-formatting branch out of the other one and I didn’t notice.

Exactly. It will take the commits on the branch you diverged from.

[thejackshelton]

Finally, another subtle change this PR includes is moving the type declarations that were inside the custom-types folder in the src of libs/qwikdev-astro to the env.d.ts of the same folder. Whenever you need to declare custom global types I think that's where they are supposed to go.

Yeah this was added recently in #69. @FloezeTv if you have insight into any drawbacks of moving it into env.d.ts don't hesitate to let us know!

[thejackshelton]

It can be done by adding --apply to the pre-commit hook, on the executed Biome command, at file lefthook.yaml.

Shouldn't this be in two places? Both the lefthook.yml and the package.json script.

[thejackshelton]

Reviewed it a couple times and happy to merge 😄 . I will create a separate commit with some small clarity changes to the contributing guide.

[FloezeTv]

@FloezeTv if you have insight into any drawbacks of moving it into env.d.ts don't hesitate to let us know!

I honestly don't know where such custom types should go. From what I can read on the Astro documentation, env.d.ts can (should?) be used for vite client-types or for extending globalThis/window. Since I can't find any other definite documentation about where to put such custom types, I guess you can put them anywhere as long as it is in a path picked up by typescript (i.e., configured in tsconfig.json) depending on your preference.

[iivvaannxx]

Happy to use your best judgement here! If we do run into this case where we want to change a config option, think this is something we should open a github discussion for?

@thejackshelton Yeah! That would probably be the best place to discuss something like that.

Pre-commit hook is great! So your preferred approach is it prevents you from committing along with a warning if there are any formatting errors?

Exactly, this approach allows you to identify any linter errors or formatting issues firsthand and correct them yourself. It prevents Biome from making 'changes' that you might be unfamiliar with later on. Although Biome only changes if it knows that it can be "safely" changed. Those are usually pretty trivial, but still, I prefer to do it myself.

With this approach, if you have any linting or formatting error, the commit will be interrupted and the lefthook CLI warns you about it.

Some things I've noticed btw, is doing something like divq (a syntax error) in a tsx file will get biome to shout at us, but it does not do the same in an Astro file 🤔. I guess this would be the partial support that biome mentioned.

That's just it, right now Biome is only able to parse the code between fences inside astro files. This is the TypeScript/JavaScript code between the --- at the beginning of the file. The markup parsing is still not supported.

type="button" on the button was interesting haha.

It's funny 😂, Biome and many other linters warn you about writing that. I think it's for accessibility, but I am not sure.

Shouldn't this be in two places? Both the lefthook.yml and the package.json script.

It would only be needed in the lefthook.yml config file, which is the one that contains the command that executes just before the commit.

The package.json has both commands, the one that only "checks" which is check, and the one that checks and applies the "safe" changes which is fix.

[iivvaannxx]

I honestly don't know where such custom types should go. From what I can read on the Astro documentation, env.d.ts can (should?) be used for vite client-types or for extending globalThis/window.

I am not a TypeScript expert, but as far as I know you usually write declaration files (these are .d.ts) to extend objects or declaring types of modules that do not contain their own types by default, like in the case of fs-move.

Usually these are "ambient" (I think they are called) type declarations and they are global, this means TypeScript picks them up automatically without you needing to import them. Many frameworks like Next, Svelte or also Astro let users use them to extend their own types or, as @FloezeTv said, extending objects like window.

Astro may say in the docs that you should only use them for certain things, but nothing prevents you from creating your own .d.ts and including it in the include TypeScript config, which would make it behave exactly the same.

I just used the already existing .d.ts to avoid creating another one, Didn't find it necessary.

@thejackshelton One problem of doing this is that if you try to explicitly use a type defined globally like that without importing it, Biome flags it as an error of "undeclared variable" or something like that. In this case it wasn't a problem because we weren't explicitly using the types that were defined in there, TypeScript picked them automatically, but is something to be aware of.

I am not sure if it's a bug or expected behaviour, if it disrupts me in any other of my projects I'll probably file an issue to the Biome repository.