Use ESLint and Prettier #27

ota-meshi · 2020-04-04T06:15:40Z

Which issue, if any, is this issue related to?

Close #12

Is there anything in the PR that needs further explanation?

This PR introduces linter and formatter to the repository.

When @stylelint/prettier-config is released, use it.
When a new version of eslint-config-stylelint is released, use it.

jeddy3

Thanks!

jeddy3 · 2020-04-04T07:37:10Z

.eslintrc.js

@@ -0,0 +1,13 @@
+module.exports = {


Let's standardise how we do this across projects, like so:

// package.json { "scripts": { "format": "prettier . --write", "lint:formatting": "prettier . --check", "lint:js": "eslint . --ignore-path .gitignore", "lint": "npm-run-all --parallel lint:*", "release": "np", "test": "jest", "watch": "jest --watch" }, "eslintConfig": { "extends": [ "stylelint" ] }, "husky": { "hooks": { "pre-commit": "lint-staged" } }, "lint-staged": { "*.js": "eslint --cache --fix", "*.(js,md,yml}": "prettier --write" } }

Integrating Prettier into ESLints floods our editors with squiggly red lines for unimportant formatting issues.

Thank you for your review!

I made this change.

Thanks for adding the linting run-script commands.

Integrating Prettier into ESLint floods our editors with squiggly red lines for unimportant formatting issues.

Can you:

replace eslint-config-prettier with a npm run format command

move the eslint config into package.json

So that it is consistent with the other stylelint repos.

Like so:

// package.json { "scripts": { "format": "prettier . --write", "lint:formatting": "prettier . --check", "lint:js": "eslint . --ignore-path .gitignore", "lint": "npm-run-all --parallel lint:*", }, "eslintConfig": { "extends": [ "stylelint" ], parserOptions: { ecmaVersion: 2019, }, rules: { 'jest/valid-expect': 'off', } } }

I moved the eslint configuration to package.json.

However, eslint-config-prettier is required. This prevents conflict between prettier and eslint. Specifically, it conflicts with "no-mixed-spaces-and-tabs".

Sorry, my bad. I mixed up the prettier plugin with the prettier config.

We should move eslint-config-prettier into eslint-config-stylelint, like we intend to do with the equivalent remark preset in @stylelint/remark-preset, as it's applicable to all stylelint repos.

We can do this in a follow-up pull request once we release a new version of eslint-config-stylelint, though.

jeddy3 · 2020-04-04T07:39:39Z

.prettierrc.js

@@ -0,0 +1,23 @@
+'use strict';


I'd prefer we use Prettier without any settings as we offload any decision making to that team (see version Prettier 2 changes) and I don't believe there's an easy way to share a Prettier config across repos, which means we have to keep them in sync manually.

I don't believe there's an easy way to share a Prettier config across repos

Scratch that: https://prettier.io/docs/en/configuration.html#sharing-configurations

If the general consensus is to define our own settings then let's pull them out into @stylelint/prettier-config and roll them out across the org. We can do that in a follow-up pull request, one that probably uses remark-preset-lint-stylelint too.

Pull request pending at stylelint/prettier-config#1

jeddy3 · 2020-04-04T12:16:17Z

.npmignore

@@ -1,14 +1,18 @@
 *.log
 *.pid


I'll create a follow-up PR to move to a files: [] whitelisting approach.

jeddy3

Thanks for making changes. I've requested a follow-up one.

jeddy3 · 2020-04-04T12:20:55Z

.eslintrc.js

@@ -0,0 +1,13 @@
+module.exports = {


Thanks for adding the linting run-script commands.

Integrating Prettier into ESLint floods our editors with squiggly red lines for unimportant formatting issues.

Can you:

replace eslint-config-prettier with a npm run format command

move the eslint config into package.json

So that it is consistent with the other stylelint repos.

Like so:

// package.json { "scripts": { "format": "prettier . --write", "lint:formatting": "prettier . --check", "lint:js": "eslint . --ignore-path .gitignore", "lint": "npm-run-all --parallel lint:*", }, "eslintConfig": { "extends": [ "stylelint" ], parserOptions: { ecmaVersion: 2019, }, rules: { 'jest/valid-expect': 'off', } } }

jeddy3

Looking good. One request and one query.

jeddy3 · 2020-04-04T12:52:56Z

.prettierrc.js

+	endOfLine: 'lf',
+	printWidth: 100,
+	singleQuote: true,
+	trailingComma: 'es5',


We use trailingComma: 'all' in the main repo. Which do we want to standardise on and use here (and in @stylelint/config-prettier)?

Currently released eslint-config-stylelint uses ES6, so trailingComma of function argument cannot be used.
https://github.com/stylelint/eslint-config-stylelint/blob/11.1.0/eslintrc.js#L12

Specify ecmaVersion: 2019 in individual eslintConfig?

Let's release a new version of eslint-config-stylelint with ecmaVersion: 2019.

In the meantime, let's offload the decision about trailing commas to Prettier and use their default (which as of version 2 is es5):

Change default value for trailingComma to es5
There is a possibility that the default value will change to always (meaning even more trailing commas) in a future major version of Prettier as the JavaScript ecosystem further matures.

Please remove trailingComma: 'es5' from this pull request and I'll remove trailingComma: 'all' from @stylelint/prettier-config.

I have removed trailingComma from .prettierrc.

jeddy3 · 2020-04-04T12:54:45Z

package.json

+    ]
  },
  "scripts": {
+    "lint:formatting": "prettier . --check",


We should add a "format": "prettier . --write", command for consistency with the other stylelint repos. Therefore, regardless of the underlying tool (for example, Jest or Mocha), the same main commands work across repositories: format, lint, test, watch, release.

…atch"

ota-meshi · 2020-04-04T13:18:42Z

I have made the changes you requested.

jeddy3

Almost there. One last amend and I think the pull request is ready.

jeddy3 · 2020-04-04T13:48:38Z

package.json

+    "lint:formatting": "prettier . --check",
+    "lint:js": "eslint .",
+    "lint": "npm-run-all --parallel lint:*",
+    "pretest": "npm run lint",


Let's remove the pretest lifecycle command and replace with a "lint" task, like so as we only want lint once on the CI. This will be consistent with other repositories.

I removed the 'pretest' script and added lint to CI.

jeddy3

Thank you!

The CI looks great with one lint run and two test runs.

jeddy3 suggested changes Apr 4, 2020

View reviewed changes

Use eslint and prettier

ad4ab04

ota-meshi force-pushed the lint branch from dd1b0e9 to ad4ab04 Compare April 4, 2020 11:40

jeddy3 reviewed Apr 4, 2020

View reviewed changes

jeddy3 suggested changes Apr 4, 2020

View reviewed changes

moved the eslint configuration to package.json.

5fba90b

jeddy3 mentioned this pull request Apr 4, 2020

Add initial config stylelint/prettier-config#1

Merged

jeddy3 suggested changes Apr 4, 2020

View reviewed changes

Update trailingComma to "all" and add npm scripts for "format" and "w…

3cc8e84

…atch"

Remove trailingComma from .prettierrc.js

7f4932a

jeddy3 suggested changes Apr 4, 2020

View reviewed changes

Remove 'pretest' script and add lint to CI

5aa3b35

jeddy3 approved these changes Apr 4, 2020

View reviewed changes

ota-meshi mentioned this pull request Apr 4, 2020

Use eslint-config-prettier stylelint/eslint-config-stylelint#82

Merged

jeddy3 merged commit 6f3f310 into master Apr 5, 2020

jeddy3 deleted the lint branch April 5, 2020 07:04

Use ESLint and Prettier #27

Use ESLint and Prettier #27

Uh oh!

Conversation

ota-meshi commented Apr 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeddy3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeddy3 Apr 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeddy3 Apr 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeddy3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeddy3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeddy3 Apr 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ota-meshi commented Apr 4, 2020

Uh oh!

jeddy3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeddy3 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

ota-meshi commented Apr 4, 2020 •

edited

Loading

jeddy3 left a comment •

edited

Loading

jeddy3 Apr 4, 2020 •

edited

Loading

jeddy3 Apr 4, 2020 •

edited

Loading

jeddy3 Apr 4, 2020 •

edited

Loading