feat: add prettier & lint-staged hook

[43081j]

Introduces a format:fix script to apply prettier changes, and a format:check script to check them.

Also introduces lint-staged such that staged files will be auto-formatted.

If you already had plans for how to introduce a formatter, happy to discard this. Just let me know.

[asyncLiz]

This honestly might be something we need to do ourselves since the external formatter has to exactly match our internal one and its settings.

Right now I believe we use clang, but there's an opportunity for us to swap out to prettier internally. We may want to do that first

[43081j]

Sure that makes sense. I know clang was used a lot in the past with the Google config, maybe that is easier to move to than prettier?

It would be helpful for contributions since you're the only ones able to have proper formatting right now I think

[asyncLiz]

We also need a contribution guide 😅

[e111077]

IIRC google is moving to prettier. I think we might have to opt-in internally though

[e111077]

update: internally our repo has moved to prettier already

[43081j]

does that mean this is a useful pr @e111077 ? or that you already have it setup internally and don't need this? 👀

[asyncLiz]

This would be very very useful

Thank you for your patience while we get to this! These kind of project infra things are critical, yet always bumped to the bottom of the priority list 🫠

Struggles of a tiny team 😞

[43081j]

no worries, i'll keep it caught up. no rush!

[e111077]

Can we move away from a git precommit action and towards a CI approach here? Also would it be able to do a prettier pass on the Sass files as well?

[43081j]

@e111077 i've done the following:

• rebase on main
• removed husky and made format:fix a dependency of build
• updated the pattern to match {scss,ts}
• added a prettierrc that matches google's style afaict
• run the formatter

[asyncLiz]

There may be configuration options we need to set to match the internal prettier. Once all set up, there should be no formatting diffs since these files are already formatted internally.

[43081j]

There may be configuration options we need to set to match the internal prettier. Once all set up, there should be no formatting diffs since these files are already formatted internally.

i totally wasn't aware of the bracketSameLine option in prettier, that seems to have sorted most of them

not sure which option, if any, sorts the one-param-per-line change. also, do you internally already format the SCSS files? those don't seem to be configurable

[43081j]

@asyncLiz @e111077 there's a couple of conflicting formats that lead me to believe it wasn't run internally in some places:

• bracket spacing is inconsistent, it seems in most places we follow the google style of {noSpacing} rather than { spacing }
• HTML start tag closing is inconsistent, in most places we have > on the same line as the last attribute, but in some places it comes after
• trailing commas mostly do exist, but in some places don't

in those cases, i chose the common one and so some formatting changes did happen

[e111077]

It is likely we may have missed some files as it will format per edited file. We can try checking this PR internally to see if we run into any inconsistencies

[e111077]

Hmm maybe internal prettier may simply be incompatible with external. Here are my findings from a quick go-over of the code

Import spacing

Ah IIRC the { spacing } is a holdover from clang format.

start tag end

Can confirm start closing tag should be on same line as last attribute unless it is preserving whitespace. e.g.

<div
  >Home</div
>

preserves whitespace of Home but

<div
  > Home</div
>

would format to

<div>
  Home</div
>

IDK if that is a configurable flag.

trailing commas

From what I see, trailing commas are simply just missing in our code.

imports

Though i found that some import lines exceed the line limit of (i think 100 chars) then the imports should wrap.

[43081j]

IDK if that is a configurable flag.

i don't think it is configurable beyond deciding if it goes on the line after or the same line

currently i have it set to "same line"

From what I see, trailing commas are simply just missing in our code.

when i run it with trailing commas off, it has a lot of changes iirc. but when i run it with trailing commas on, it still has changes, just not as many 😬

Though i found that some import lines exceed the line limit of (i think 100 chars) then the imports should wrap

afaik prettier tries to wrap to the column limit but sometimes exceeds it for whatever reason. not sure there's anything we can do about that

[asyncLiz]

I'm not sure how we can push this forward. If external prettier formats differently than internal prettier, our formatting tests are going to fail on both sides as each one wants to format it differently.

I haven't looked at our internal formatter, but it's quite possible it's customized. We'd need to see what internal prettier is doing to reflect it externally too.

[asyncLiz]

It does look like we have a custom preprocess after prettier for TS imports. It re-formats them using ts.OrganizeImportsArgs from the typescript api.

We also add prettierConfig.embeddedLanguageFormatting = 'auto' for lit-importing files.