facia-tool icon indicating copy to clipboard operation
facia-tool copied to clipboard

Run yarn lint and prettier in CI

Open emdash-ie opened this issue 5 months ago • 1 comments

What's changed?

We in Content Pipeline have had some trouble with different people’s editors disagreeing about formatting, so I’d like to run formatting checks in CI to keep it consistent.

There’s a little more info in the commit messages, reproduced here:

f1e9664413 Run yarn lint in CI

We’ve had some trouble with different people’s editors disagreeing about formatting, so I’d like to make our formatting checks in CI to have it consistent.

5c8fde87f4 Update @typescript-eslint/parser

I was getting a warning that our version of typescript is too new for our version of this package: updating removes the warning.

b3d894b598 Replace tslint with typescript-eslint, updates

TSLint is deprecated, and the suggested replacement is typescript-eslint.

This commit also updates eslint, prettier, and eslint-plugin-prettier, and adds eslint-config-prettier.

64dbdbd912 Run prettier directly rather than via eslint

Prettier’s docs recommend it be run directly rather than via eslint, so this commit removes eslint-plugin-prettier and runs it directly within the lint scripts in package.json.

ed4d99b2a9 Use @guardian/prettier for prettier config

6f84c63b41 Fix .prettierrc symlink in root

I’m guessing the path to fronts-client was updated, and the symlink wasn’t.

2297df1e29 Format everything with prettier

To produce this diff, I ran yarn lint-fix.

c47958b698 Update .editorconfig files to match prettier

The @guardian/prettier config uses tabs for indentation, so this commit changes the .editorconfig files to match.

(Since the .prettierrc is linked at the root, I think I need to update both .editorconfig files.)

3714938840 Format tables nicely in readme

My editor has some auto-formatting that makes these tables much nicer to read in the markdown source: unless anyone objects I figure it’s an improvement?

9a4f829e33 Update readme

  • remove mentions of TSLint
  • recommend setting up editor integration for Prettier

793db33d4f Steal isError from tslint

Now that I’ve removed TSLint, this import is failing the build. To fix the build, this commit copies the definition of isError from my local node_modules.

I took a look at https://github.com/guardian/facia-tool/pull/1277, which introduced this, and I didn’t see any explanation for this bit of code: it seems strange to import something from tslint (which is a dev dependency) for this, which makes me wonder if it was deliberate?

Either way, I believe the check is necessary: I initially tried removing it, but that broke some tests.

d9f2322ee5 Remove tslint comments

Now that we’re not using TSLint, these comments aren’t doing anything.

I’ll reintroduce the equivalents for eslint if they’re required.

Implementation notes

  • There are some styling changes included in this: for example, the @guardian/prettier config that I’ve configured prettier to use uses tabs for indentation rather than spaces.
    • The config links to https://alexandersandberg.com/articles/default-to-tabs-instead-of-spaces-for-an-accessible-first-environment/ and https://x.com/Rich_Harris/status/1541761871585464323 as explanations of the reason for choosing tabs
  • The prettier formatting is all in one commit to make it easier to see what’s changed that isn’t formatting.

Checklist

General

  • [ ] 🤖 Relevant tests added
  • [ ] ✅ CI checks / tests run locally
  • [ ] 🔍 Checked on CODE

Client

  • [ ] 🚫 No obvious console errors on the client (i.e. React dev mode errors)
  • [ ] 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
  • [ ] 📷 Screenshots / GIFs of relevant UI changes included

emdash-ie avatar Sep 30 '24 14:09 emdash-ie