Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

Add support ESLint 8 #137

Merged
merged 6 commits into from
Jan 5, 2022
Merged

Add support ESLint 8 #137

merged 6 commits into from
Jan 5, 2022

Conversation

sndrs
Copy link
Member

@sndrs sndrs commented Dec 13, 2021

What does this change?

  • upgrades to ESLint 8
  • upgrades to prettier 2.4
    • jsxBracketSameLine is deprecated
  • makes eslint an explicit peerDep of @guardian/eslint-config-typescript
  • pins @guardian/eslint-config-typescript's dependency on @guardian/eslint-config to a specific version
  • peerdeps more accurately express the widest possible range
  • installs the minimum version of peerdeps we support as dev deps
  • removed unnecessary devDeps

also:

  • use yarn workspaces (rather than lerna's)
    • this give us better feedback from yarn about peerDep irregularities

@sndrs sndrs requested a review from a team as a code owner December 13, 2021 12:07
@sndrs sndrs requested a review from akash1810 December 13, 2021 12:08
"lerna": "^4.0.0",
"prettier": "^2.3.1",
"typescript": "^4.3.2"
"prettier": "2.4.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: is there a reason we must pin to this specific version of prettier, or can we specify a ^ range in keeping with other devDeps?

Copy link
Member Author

Choose a reason for hiding this comment

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

2.4 has the new brackets flag config, so this becomes the minimum version the workspace should work with

Copy link
Contributor

Choose a reason for hiding this comment

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

I get ya. So the general rule is:

  • in peerDependencies, specify the minimum supported version of a package, using a ^ to pin the major version and enforce a minimum minor version
  • in devDependencies, specify the minimum supported version of a package, using the exact version (i.e. no ^ or ~)

Copy link
Member Author

Choose a reason for hiding this comment

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

that's was the thinking yeah – make it as flexible as possible to consume, but make sure we develop with the smallest possible feature set we need. and then obvs if we need a feature from a later version that becomes the baseline

@@ -13,7 +13,7 @@
"url": "https://github.com/guardian/configs/issues"
},
"peerDependencies": {
"prettier": "^2.0.0"
"prettier": "^2.4.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why force consumers to upgrade? Are we using features that were added after 2.0.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah the new new brackets config comes with 2.4

"eslint-plugin-eslint-comments": "^3.2.0",
"eslint-plugin-import": "^2.23.4",
"eslint-plugin-prettier": "^3.4.0",
"eslint": "8.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

8.4.1 is the latest, should we be using that instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea was to install the minimum possible version in the workspace that our packages should work with, to make using them as flexible as possible without accidentally using features added after the minimum version

Copy link
Contributor

@SiAdcock SiAdcock left a comment

Choose a reason for hiding this comment

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

This is cool. I think it'll be a breaking change, so major bump when we publish?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants