Adding a formatter for CSS & JS

[tom]

I was talking to @thibaudcolas and he mentioned the idea of adding formatting for CSS and JS, as well as linting for CSS. I think the easier sell here is the formatter, so I’ll propose that here.

For motivation, I find myself fairly often in the admin CSS (especially lately) and it’s reasonably laid out for the most part, but there are things that are a little off. Sometimes the indentation is different from the rest of the file, for example. The JS is in a bit worse shape I think, some of the files have whitespace issues. These could be fixed but it would be better to enforce them in a way that’s easier to review, so I think a formatter makes sense here. What’s annoying (for me) is that when editing files, sometimes my editor “fixes” the issues for me, then I have to revert them and save without formatting. If I don’t spot my editor’s work, it’s more annoying to fix. Of course I could turn this stuff off somewhere, but if it’s annoying for me I’m sure it’ll be annoying for others as well.

I’m open to suggestions, but Thibaud suggested prettier, and I also use it myself. It doesn’t really require any configuration, though is configurable if people do have strong opinions.

But barring differing opinions there, the process would be something like:

Make a PR with three commits. The first sets up the machinery, installs prettier, adds to pre-commit, etc. The second does the work and the third adds the second commit to .git-blame-ignore-revs. As far as I know, the same as when black was set up.

Probably it would be better to do CSS and JS in two separate PRs.

[carltongibson]

We have ESLint in play for the JS. I think that’s been a net-positive. Flake8 catches a lot.

Black has been a win. Auto-formatters generally are great IMO

So +1 from me, in principle.

[adamchainz]

+1 Let’s Prettier it up!

[thibaudcolas]

A +1 here too, as Tom mentioned we’ve already discussed this idea quite a bit.

On the process, I’d be interested to hear why you think it’d be better separately for CSS and JS.

---

Thinking of a few possible things to watch out for,

• The ESLint configuration seems to have a few enabled rules about formatting, which might need to be changed to be compatible with Prettier formatting, or need disabling. I’d recommend disabling and leaving all formatting concerns to Prettier.
• One of Prettier’s features is adding trailing commas in function calls and parameter definitions, which wasn’t valid syntax until ES2017. That was a while ago now so hopefully not a problem – but something to watch out for if we have automated processes running an outdated JS engine, or static analysis of some kind with an outdated parser.
• If Django uses “ignore comments”, we’ll need to check they’re still targeting the correct lines after the reformatting. I only spot a single eslint-disable right now but there could be other tools I’m not aware of.
• Prettier is generally pretty fast but it’d be nice to check if there are large folders it might try to crawl looking for files for no reason, and if so ignore those explicitly
• As part of merging this, be nice to give a heads’up to code contributors so they’re aware they might see unexpected results / might want to install the relevant code editor integrations.

[tom]

@thibaudcolas

On the process, I’d be interested to hear why you think it’d be better separately for CSS and JS.

Just to ease the review burden. I also think CSS might be easier to land (not as many strong opinions around formatting as can come up with a full programming language).

@thibaudcolas

I’d recommend disabling and leaving all formatting concerns to Prettier.

Agreed.

@thibaudcolas

but there could be other tools I’m not aware of.

I don’t believe there are.

@thibaudcolas

Prettier is generally pretty fast but it’d be nice to check if there are large folders it might try to crawl looking for files for no reason, and if so ignore those explicitly

I think there are only a few directories we need to run this against. Admin, GIS, and tests (didn’t look hard for others yet). We should also not run it against the vendored CSS and JS.

@thibaudcolas

As part of merging this, be nice to give a heads’up to code contributors so they’re aware they might see unexpected results / might want to install the relevant code editor integrations.

Maybe, but how? Did we do this for black? Don’t recall and wasn’t really involved. Perhaps a follow-up forum post is enough?

[carltongibson]

@thibaudcolas

As part of merging this, be nice to give a heads’up to code contributors so they’re aware they might see unexpected results / might want to install the relevant code editor integrations.

This is my main Yes, please. A good How to set it up in the contributing guide is both welcome and (I think) essential.

[tom]

This is just my opinion.

As long as the docs cover setting up pre-commit, which is already the case, it should be enough. One of the nice things about formatters is that you don’t have to worry about your own formatting and can just throw code at your editor until it works, then rely on pre-commit to clean it up for you.

A note on how to run it manually might also be useful for some. For example, on ESLint, we have:

Use the ESLint code linter to check your code for bugs and style errors. ESLint will be run when you run the JavaScript tests. We also recommended installing a ESLint plugin in your text editor.

I think something like this should be enough (maybe it can also run with the JS tests).

I think docs for specific editor integrations is probably too difficult to maintain with the shifting landscape and vast amount of editors in popular use.

[carltongibson]

Surely you want edit on save and all that jazz, so you need to install separately to just pre-commit?

[tom]

I don’t think this is so surely. I like to set it up this way myself, but I know a lot of people just write code and rely on pre-commit to do the formatting, rather than battle with editor extensions.

[nessita]

I’m +1 to CSS and JS auto formatters! Thanks for raising this topic.

[carltongibson]

@tom

I don’t think this is so surely.

Indeed. (When I was an undergrad, I was taught that Surely doesn’t mean certainly, but rather No argument will be presented for the following claim — I use it rhetorically in the latter sense )

I think docs should show how to install and run the relevant command… for (e.g.) Black…

pip install black
black .

… or similar. That’d do.

I’m not suggesting we get into editor specifics

[smagucha]

Plus 1 let’s do it. I completely support you.

[matthiask]

This is a great idea. ESLint is already configured (a bit) in .pre-commit-config.yaml.

Something like exclude: "vendor" at the top of the pre-commit configuration should also take care of not checking and reformatting jquery etc.

I always use eslint-config-prettier to automatically turn off ESLint rules which are useless when using prettier. It works well.

[thibaudcolas]

I count six “+1s” and no other “votes” cast, so I was about to create a ticket for this – but perhaps @tom you have other plans?

I don’t remember whether your participation in a high-profile Django mentoring program has been publicly announced or not – does this work feel like the kind of thing you might want to earmark for a participant to said program?

If not, based on what you said, should we create separate tickets for CSS and JS or just the one?

---

Edit: for context, here’s an example of stylesheets rework where auto-formatting would help: PR #17560 – [RFC/WIP] Refs #??? – Switched to logical properties in CSS. started by @ngnpope.

[tom]

I added a ticket already: #35007 (Add prettier to format CSS and JS) – Django

I plan to work on it soonish, hopefully this week, but certainly this year.

@thibaudcolas

I don’t remember whether your participation in a high-profile Django mentoring program has been publicly announced or not – does this work feel like the kind of thing you might want to earmark for a participant to said program?

It’s plausible, hadn’t thought about this in particular. Depends if I find time to work on it before then or not.

[adamchainz]

Another option for JS formatting is Standard JS, which is basically a Black-style “like it or lump it” formatter + linter, implemented as a wrapper around ESLint. I swapped a couple of projects over and like it, and I’m gonna update Boost Your Django DX to recommend it over plain ESLint.

(But I’m happy with Prettier + ESLint for Django too.)

[thibaudcolas]

I don’t think Standard is suitable for what we’re discussing here, as we’re after a formatter rather than linting. Standard does have formatting-related linting rules but they’re very limited compared to what Prettier does. And yeah it’s based on ESLint, which itself has deprecated all formatting rules (their formatting rules were created at a time when auto-formatting didn’t exist, and they now recommend using a real formatter instead).

It could be that Standard decides to use the linting rules anyway, extracted as a separate package that they might bundle, but still – those rules are way more limited than what Prettier does.

[tom]

Okay, so I’ve made a start on this, and have a working configuration.

The problem comes with adding this to pre-commit, which has recently archived the prettier mirror apparently because prettier has broken the ability to use it.

Prettier itself provides some pre-commit options but it would be the first time for us adding something like this.

Of the options available…

Option 1, lint-staged with husky seems reasonable and wouldn’t add too much extra.

Option 2 requires .NET so seems a non-starter.

Option 3 seems okay but is confusing to me.

Option 4 also looks like a non-starter because it lives in the .git directory and can’t be version controlled.

Are there any objections to going with their option 1 here? I don’t really want to have another tool for managing pre-commit hooks, especially when pre-commit is supposed to be able to do everything… but if it can’t, then… I am not sure what good alternative there is.

P.S. looking at Issue with pretier 4.0.0@alpha3 and pre-commit · Issue #15742 · prettier/prettier · GitHub it seems like the pre-commit mirror should still work, the issue that caused the archival was only temporary, so I suppose if someone forked this and updated it, it could be used. Sadly based on his past behaviour I don’t think we can rely on Anthony unarchiving the project any time soon (or ever).

[thibaudcolas]

If we don’t want extra tools, I would assume running a local hook would be simpler than options 1 through 3? Here’s an example local hook for Prettier:

  # Local hooks that require dependencies to be installed already.
  - repo: local
    hooks:
      - id: prettier
        name: prettier
        language: system
        entry: node_modules/.bin/prettier --write
        types_or:
          - css
          - javascript

Drawback compared to a mirror is this requires having the project’s Node dependencies installed locally. Advantage is no duplication of tooling versions compared to asottile’s pre-commit.

Or we can use a fork of the mirror (existing or make a new one). I’ve been maintaining a stylelint mirror for a while so happy to help / advise.

[matthiask]

I have been using npx to avoid the need to install packages locally into the project:

  - repo: local
    hooks:
      - id: prettier
        name: prettier
        entry: npx prettier@3.3.3 --no-semi --write
        language: system
        types_or: [css, scss]
        require_serial: true

GitHub - biomejs/pre-commit: Biome's pre-commit hooks. would be a much faster alternative, but maybe less established than prettier.