Skip to content
This repository was archived by the owner on Apr 23, 2025. It is now read-only.

Initial implementation of shared eslint config #1

Closed
bajtos wants to merge 2 commits into from
Closed

Initial implementation of shared eslint config #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3369794
Initial implementation
bajtos Apr 10, 2025
880dadb
remove config specific to CheckerNetwork/node
bajtos Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
* @bajtos @juliangruber @pyropy @NikolasHaimerl
package.json
package-lock.json
11 changes: 11 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
version: 2
updates:
- package-ecosystem: 'npm'
directory: '/'
schedule:
interval: 'daily'
time: '09:00'
timezone: 'Europe/Berlin'
commit-message:
prefix: 'deps'
prefix-development: 'deps(dev)'
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: CI

on:
push:
branches: [main]
pull_request:

jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
- run: npm ci
- run: npm run lint
28 changes: 28 additions & 0 deletions .github/workflows/dependabot-auto-approve-minor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: Dependabot auto-approve minor updates
on: pull_request

permissions:
pull-requests: write

jobs:
dependabot:
runs-on: ubuntu-latest
if: ${{ github.actor == 'dependabot[bot]' }}
strategy:
matrix:
dependencyStartsWith:
- prettier
- neostandard
- eslint
steps:
- name: Dependabot metadata
id: metadata
uses: dependabot/fetch-metadata@v2
with:
github-token: '${{ secrets.GITHUB_TOKEN }}'
- name: Approve a PR
if: ${{startsWith(steps.metadata.outputs.dependency-names, matrix.dependencyStartsWith) && (steps.metadata.outputs.update-type == 'version-update:semver-patch' || steps.metadata.outputs.update-type == 'version-update:semver-minor')}}
run: gh pr review --approve "$PR_URL"
env:
PR_URL: ${{github.event.pull_request.html_url}}
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
17 changes: 17 additions & 0 deletions .github/workflows/dependabot-auto-merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Dependabot auto-merge
on: pull_request

permissions:
contents: write
pull-requests: write

jobs:
dependabot:
runs-on: ubuntu-latest
if: ${{ github.actor == 'dependabot[bot]' }}
steps:
- name: Enable auto-merge for Dependabot PRs
run: gh pr merge --auto --squash "$PR_URL"
env:
PR_URL: ${{github.event.pull_request.html_url}}
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
node_modules
21 changes: 21 additions & 0 deletions .prettierrc.js
View file
Open in desktop
Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 15, 2025

Choose a reason for hiding this comment

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

Alleviating your concerns about PR size a bit, this file can be removed as we make this repo also depend on the shared prettier config

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @typedef {import('prettier').Config} Config
* @see https://prettier.io/docs/configuration
*/

/** @type {Config} */
export default {
plugins: [
// Order of plugins is important!
// See https://github.com/electrovir/prettier-plugin-multiline-arrays?tab=readme-ov-file#compatibility
'prettier-plugin-packagejson',
'prettier-plugin-jsdoc',
],

// built-in options
semi: false,
singleQuote: true,

// prettier-plugin-jsdoc
tsdoc: true,
}
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,11 @@
# eslint-config

Shared configuration for ESLint based on Neostandard
Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 15, 2025

Choose a reason for hiding this comment

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

This is missing docs for how to use this config, could you add that too?


## Development

Publish a new version:

```bash
$ npx np
```
6 changes: 6 additions & 0 deletions eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import neostandard from 'neostandard'

export default neostandard({
noStyle: true, // Disable style-related rules, we use Prettier
ts: true,
})
Comment on lines +3 to +6
Copy link
Copy Markdown
Member Author

@bajtos bajtos Apr 10, 2025

Choose a reason for hiding this comment

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

I am not convinced that sharing a two-line configuration is worth the extra ceremony required. (This patch has +4,949 new lines.)

We will also need to remember to periodically publish new versions of this package in order to ship neostandard updates to our repositories.

@juliangruber WDYT?

Copy link
Copy Markdown
Member Author

@bajtos bajtos Apr 10, 2025

Choose a reason for hiding this comment

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

package-lock.json is adding 4,808 lines, so the actual meaningful content is "only" 141 lines. To share eslint.config.js containing six lines of code 🤔

Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 15, 2025

Choose a reason for hiding this comment

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

I don't see it this way, the PR is already there and the extra code is basic orchestration. There are many repositories where the extra code is so much more than the actual code. We might also develop the eslint config in the future, or perform tooling upgrades. We don't want to do this in every repo one by one.

Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 15, 2025

Choose a reason for hiding this comment

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

I do wonder though if there's a better way to share this config

Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 15, 2025

Choose a reason for hiding this comment

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

For one, can we share the eslint and prettier config from one repo? This way there's half the orchestration, and less updates. It gets closer to my desire of having just one package for all things listing & formatting.

Copy link
Copy Markdown
Member Author

@bajtos bajtos Apr 17, 2025

Choose a reason for hiding this comment

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

I was thinking about this quite a bit.

Neostandard is already the shared linter config that shields us from the details of eslint configuration. The two tweaks we need to make - enable ts and disable style - are kind of specific to each repository. I don't expect them to change unless we significantly rework our formatting & linting setup.

The situation with Prettier is more complex; we need our custom shared config. Prettier provides a neat solution for that:

{
  "prettier": "@CheckerNetwork/prettier-config"
  "devDependencies": {
    "@CheckerNetwork/prettier-config": "^1.0.0",
    "prettier": "^3.5.3"
  }
}

Is it worth innovating here? I am happy to follow the default Prettier approach.

For one, can we share the eslint and prettier config from one repo?

It's not the typical path supported by Prettier.

I see two potential options we can explore:

  • In package.json, add `"prettier": "@CheckerNetwork/lint-config/prettier" - if Prettier supports importing from package sub-paths
  • Use .prettierrc.js file that re-exports only the Prettier part of the shared configuration

I am not convinced the benefit of having one repo combining eslint and Prettier config is worth deviating from the typical path, though. These two configurations are independent (as long as we keep eslint style-rules disabled).

Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 17, 2025

Choose a reason for hiding this comment

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

Got it, if that's not supported, let's not innovate. +1 to inlining the neostandard config, and using one repo for the prettier config

Copy link
Copy Markdown
Member

@juliangruber juliangruber Apr 17, 2025

Choose a reason for hiding this comment

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

I want to go the overall simplest path possible here

Loading