Lint: Disable errors for formatting handled by prettier #24635

blowery · 2018-05-02T18:49:04Z

Since we now use prettier to format all of our code, we can turn eslint formatting errors covered by prettier (max-len, indent...). Prettier provides a config to do just that in eslint-format-prettier.

~~The only real problem we have now is ternary expressions, so we could just ignore those nodes instead.~~

See #24504, #24571

matticbot · 2018-05-02T18:49:05Z

Test live: https://calypso.live/?branch=try/eslint/turn-off-indent

gziolo · 2018-05-04T07:28:41Z

.eslintrc.js

+		// REST API objects include underscores
+		camelcase: 0,
+
+		// TODO: why did we turn this off?


I think it errors because there are still chai matchers which aren't compatible with Jest syntax. I bet the same story as with mocha globals.

gziolo · 2018-05-04T07:29:59Z

.eslintrc.js

 	parser: 'babel-eslint',
 	env: {
 		browser: true,
 		'jest/globals': true,
+		// mocha is only still on because we have not finished porting all of our tests to jest's syntax
 		mocha: true,


You can try to turn it off as we most likely migrated all globals to Jest.

sure, different PR for that though.

blowery · 2018-05-10T20:09:52Z

ok, now we really really use prettier for everything

#24752

thanks @sirreal!

sirreal

Excited to remove an annoyance with this 🙂

Added a comment about max-len. If our goal here is "prettier handles that, be silent eslint" PR, then I'd like to remove max-len as well.

Commented a few of the confusing parts of the diff so it's clear what rules aren't changed but have been formatted.

sirreal · 2018-05-11T07:02:47Z

.eslintrc.js

+		'jsx-a11y/anchor-has-content': 0,
+
+		// error if any given line is over 140 characters long.
+		// Ideally this would be even lower(120, 80?), but that's a big step.
 		'max-len': [ 2, { code: 140 } ],


Shall we disable this as well? Prettier's functionality is based on breaking long lines.

A quick search will show lots of disabling for SVG paths or strings that aren't broken.

Would remove 98 lint errors according to #24504 (comment) and allow for removal of ~69 {en,dis}able comments

sounds good to me. I'll rename this PR.

sirreal · 2018-05-11T07:03:17Z

.eslintrc.js

-		} ],
+
+		// Allows Chai `expect` expressions. Now that we're on jest, hopefully we can remove this one.
+		'no-unused-expressions': 0,


No change, layout and comment.

sirreal · 2018-05-11T07:03:38Z

.eslintrc.js

+			{
+				rootFiles: [ 'index.js', 'index.jsx', 'main.js', 'main.jsx' ],
+			},
+		],


No change, layout and comment.

sirreal · 2018-05-11T07:04:06Z

.eslintrc.js

-	}
+
+		// turn off indenting rules since we let prettier handle it
+		indent: 0,


New rule disabling eslint indent 🎉

sirreal · 2018-05-11T07:04:29Z

.eslintrc.js

-		'jsx-a11y/anchor-has-content': 0, // i18n-calypso translate triggers false failures
+
+		// i18n-calypso translate triggers false failures
+		'jsx-a11y/anchor-has-content': 0,


No change, layout.

sirreal · 2018-05-11T07:17:01Z

I've pushed a commit that removes the now-redundant indent disable/enable comments. Feels so good!

sirreal · 2018-05-11T07:20:50Z

I've rebased. There was a new indent ignore that had been added, which is now removed 🙄

blowery · 2018-05-11T14:12:39Z

@sirreal taking a slightly different approach here. I'm going to pull in and use https://github.com/prettier/eslint-config-prettier

blowery · 2018-05-11T14:29:44Z

Adding the prettier eslint config has the following effect:
npx eslint --ext js,jsx client

--- before	2018-05-11 10:28:38.000000000 -0400
+++ after	2018-05-11 10:28:50.000000000 -0400
@@ -1,11 +1,9 @@
-indent: 1790
 wpcalypso/jsx-classname-namespace: 844
 prefer-const: 647
 react/no-string-refs: 249
 valid-jsdoc: 247
 jsx-a11y/click-events-have-key-events: 106
 jsx-a11y/no-static-element-interactions: 98
-max-len: 98
 jsx-a11y/anchor-is-valid: 69
 jsx-a11y/alt-text: 62
 jsx-a11y/label-has-for: 41
@@ -38,14 +36,12 @@
 react/no-did-mount-set-state: 3
 react/no-is-mounted: 3
 jsx-a11y/media-has-caption: 2
-no-multi-spaces: 2
 no-var: 2
 eqeqeq: 1
 jsx-a11y/html-has-lang: 1
 jsx-a11y/role-has-required-aria-props: 1
 no-nested-ternary: 1
 no-use-before-define: 1
-operator-linebreak: 1
 wpcalypso/i18n-no-variables: 1

-total: 4585
+total: 2694

samouri · 2018-05-11T21:19:54Z

Disable errors for formatting handled by prettier

This PR is doing a lot more than that right?

samouri · 2018-05-11T21:20:38Z

client/my-sites/stats/stats-first-view/index.jsx

@@ -21,21 +21,20 @@ class StatsFirstView extends React.PureComponent {
 				<div>
 					{ this.renderIcon() }
 					<h1>{ this.props.translate( 'See How Your Site Is Performing' ) }</h1>
-					{ /* eslint-disable max-len */ }
+					{  }


you can delete this { }

samouri · 2018-05-11T21:21:22Z

client/my-sites/site-settings/delete-site/index.jsx

@@ -163,7 +163,7 @@ class DeleteSite extends Component {
 				<ActionPanel>
 					<ActionPanelBody>
 						<ActionPanelFigure>
-							{ /* eslint-disable max-len */ }
+							{  }


this one too

samouri

Two small changes, but after fixing those feel free to 🚢

sirreal

I cleaned up the empty braces @samouri spotted.

Looked through code, seems correct.
Smoke tested, didn't spot any errors.

Let's 🚢 !

sirreal · 2018-05-14T07:56:54Z

client/lib/shortcode/index.js

@@ -186,7 +186,7 @@ export const regexp = memoize( function( tag ) {
 	return new RegExp(
 		'\\[(\\[?)(' +
 			tag +
-			')(?![\\w-])([^\\]\\/]*(?:\\/(?!\\])[^\\]\\/]*)*?)(?:(\\/)\\]|\\](?:([^\\[]*(?:\\[(?!\\/\\2\\])[^\\[]*)*)(\\[\\/\\2\\]))?)(\\]?)', // eslint-disable-line max-len
+			')(?![\\w-])([^\\]\\/]*(?:\\/(?!\\])[^\\]\\/]*)*?)(?:(\\/)\\]|\\](?:([^\\[]*(?:\\[(?!\\/\\2\\])[^\\[]*)*)(\\[\\/\\2\\]))?)(\\]?)',


sirreal · 2018-05-14T08:11:39Z

client/lib/shortcode/index.js

-const REGEXP_ATTR_STRING = /(\w+)\s*=\s*"([^"]*)"(?:\s|$)|(\w+)\s*=\s*\'([^\']*)\'(?:\s|$)|(\w+)\s*=\s*([^\s\'"]+)(?:\s|$)|"([^"]*)"(?:\s|$)|(\S+)(?:\s|$)/g; // eslint-disable-line max-len
-const REGEXP_SHORTCODE = /\[(\[?)([^\[\]\/\s\u00a0\u200b]+)(?![\w-])([^\]\/]*(?:\/(?!\])[^\]\/]*)*?)(?:(\/)\]|\](?:([^\[]*(?:\[(?!\/\2\])[^\[]*)*)(\[\/\2\]))?)(\]?)/; // eslint-disable-line max-len
+const REGEXP_ATTR_STRING = /(\w+)\s*=\s*"([^"]*)"(?:\s|$)|(\w+)\s*=\s*\'([^\']*)\'(?:\s|$)|(\w+)\s*=\s*([^\s\'"]+)(?:\s|$)|"([^"]*)"(?:\s|$)|(\S+)(?:\s|$)/g;
+const REGEXP_SHORTCODE = /\[(\[?)([^\[\]\/\s\u00a0\u200b]+)(?![\w-])([^\]\/]*(?:\/(?!\])[^\]\/]*)*?)(?:(\/)\]|\](?:([^\[]*(?:\[(?!\/\2\])[^\[]*)*)(\[\/\2\]))?)(\]?)/;


blowery added Framework Build [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 2, 2018

blowery force-pushed the try/eslint/turn-off-indent branch from 098f1cb to 0da0540 Compare May 2, 2018 18:51

blowery mentioned this pull request May 2, 2018

Store: Fix ESLint errors in WCS, tests, lib, and components directories #24571

Merged

blowery requested review from ockham, gziolo and a team May 2, 2018 19:44

gziolo reviewed May 4, 2018

View reviewed changes

ryelle mentioned this pull request May 9, 2018

Extensions – Zoninator: Fix eslint issues #24762

Merged

2 tasks

sirreal reviewed May 11, 2018

View reviewed changes

blowery and others added 4 commits May 11, 2018 09:19

format the lint config

9386950

ignore indent when linting

b199445

add comments explaining the config

34fee37

Remove eslint {dis,en}able indent comments

0870b52

sirreal force-pushed the try/eslint/turn-off-indent branch from 8a4baa6 to 0870b52 Compare May 11, 2018 07:20

blowery changed the title ~~Lint: Turn off indent errors~~ Lint: Disable errors for formatting handled by prettier May 11, 2018

turn off max-len too, consolidate prettier overrides

29d5b71

switch to using eslint-config-prettier to turn off formatting rules

29b76a2

remove max-len overrides

90f4317

blowery mentioned this pull request May 11, 2018

Upgrade Prettier to version 1.13.5 #24551

Merged

samouri reviewed May 11, 2018

View reviewed changes

samouri approved these changes May 11, 2018

View reviewed changes

Remove empty jsx braces (contained eslint comments)

7a4afcd

sirreal approved these changes May 14, 2018

View reviewed changes

blowery added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 14, 2018

blowery merged commit cddb367 into master May 14, 2018

blowery deleted the try/eslint/turn-off-indent branch May 14, 2018 13:52

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 14, 2018

blowery mentioned this pull request May 14, 2018

Fix all the eslint errors #24504

Closed

sirreal mentioned this pull request Jul 9, 2018

Guided Tours: Add Jetpack Sign-In Tour #25864

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lint: Disable errors for formatting handled by prettier #24635

Lint: Disable errors for formatting handled by prettier #24635

blowery commented May 2, 2018 •

edited

Loading

matticbot commented May 2, 2018

gziolo May 4, 2018 •

edited

Loading

gziolo May 4, 2018

blowery May 4, 2018

blowery commented May 10, 2018

sirreal left a comment

sirreal May 11, 2018

sirreal May 11, 2018

blowery May 11, 2018

sirreal May 11, 2018

sirreal May 11, 2018

sirreal May 11, 2018

sirreal May 11, 2018

sirreal commented May 11, 2018

sirreal commented May 11, 2018

blowery commented May 11, 2018

blowery commented May 11, 2018 •

edited

Loading

samouri commented May 11, 2018

samouri May 11, 2018

samouri May 11, 2018

samouri left a comment

sirreal left a comment

sirreal May 14, 2018

sirreal May 14, 2018

Lint: Disable errors for formatting handled by prettier #24635

Lint: Disable errors for formatting handled by prettier #24635

Conversation

blowery commented May 2, 2018 • edited Loading

matticbot commented May 2, 2018

gziolo May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blowery commented May 10, 2018

sirreal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sirreal commented May 11, 2018

sirreal commented May 11, 2018

blowery commented May 11, 2018

blowery commented May 11, 2018 • edited Loading

samouri commented May 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouri left a comment

Choose a reason for hiding this comment

sirreal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blowery commented May 2, 2018 •

edited

Loading

gziolo May 4, 2018 •

edited

Loading

blowery commented May 11, 2018 •

edited

Loading