Style error information

https://eslint.org/docs/rules/operator-linebreak

How to Review

## 1. Apply Patch
## 2. Review Code Changes
## 3. Confirm no Code Standard Errors
yarn & yarn lint:core-js-passing
## 4.1 If `NO` errors, mark the issue as `Reviewed & tested by the community` (Don't be shy, we're all friendly)
## 4.2 If `HAS` errors, fix it and upload a new patch (Just do it and you can!!!)

Background

- #2912962: Step 1 JS codestyle: [meta] Fix JS coding standards in core

- We adapted the airbnb coding standard (#2815077: Adopt airbnb javascript style guide v14.1 as new baseline javascript coding standards for Drupal 8 core and contrib), but we are not fully compliant to it yet.

More Information

- Using ES6 in Core
https://www.drupal.org/node/2815083

- To find JS code standard errors stats
cd core/ && yarn & yarn lint:core-js-stats

Valuable Follow-up

- N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drpal created an issue. See original summary.

ApacheEx’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
11.27 KB

Here is a patch. I have also run yarn run build:js.

m1r1k’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
/Volumes/devdisk/dev/drupal/core/misc/ajax.es6.js
  1035:51  error  '&&' should be placed at the beginning of the line  operator-linebreak
  1041:52  error  '||' should be placed at the beginning of the line  operator-linebreak

✖ 2 problems (2 errors, 0 warnings)
  2 errors, 0 warnings potentially fixable with the `--fix` option.

Need to update the patch due to changes to core.

m1r1k’s picture

Status: Needs work » Needs review
FileSize
12.1 KB
1.02 KB

Here is a patch. I have also run yarn run build:js

Status: Needs review » Needs work

The last submitted patch, 5: drupal-js-eslint-line-break-2983365-5.patch, failed testing. View results

m1r1k’s picture

Status: Needs work » Needs review
FileSize
12.11 KB

Haven't updated from upstream :/
Here is reroll

anmolgoyal74’s picture

Status: Needs review » Reviewed & tested by the community

Patch applied successfully.
Marking it RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: drupal-js-eslint-line-break-2983365-7.patch, failed testing. View results

ApacheEx’s picture

Rerolled + some small improvements.

To be honest this rule is weird. Not sure if we should go with it.
Even prettier prefers something like this:

if (
  (!itemId && !itemClass) ||
  (itemId && $document.find(`#${itemId}`).length) ||
  (itemClass && $document.find(`.${itemClass}`).length)
) {
  return true;
}
GrandmaGlassesRopeMan’s picture

@ApacheEx - I agree, this is strange. What about this configuration? operator-linebreak: ["error", "after"]

ApacheEx’s picture

FileSize
2.54 KB

Cool, the patch looks much cleaner.

I also guess this rule should be placed in .eslintrc.json instead of .eslintrc.passing.json

GrandmaGlassesRopeMan’s picture

Status: Needs review » Reviewed & tested by the community

@ApacheEx - 👏 Much nicer.

ApacheEx’s picture

FileSize
2.54 KB

Rerolled with the latest changes from 8.6.x

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ApacheEx’s picture

FileSize
2.52 KB

Rerolled with 8.7.x

The last submitted patch, 14: 2983365-14.patch, failed testing. View results

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed e15f82bfba to 8.7.x and 8ca7890875 to 8.6.x. Thanks!

  • alexpott committed e15f82b on 8.7.x
    Issue #2983365 by ApacheEx, m1r1k, drpal: JS codestyle: operator-...

  • alexpott committed 8ca7890 on 8.6.x
    Issue #2983365 by ApacheEx, m1r1k, drpal: JS codestyle: operator-...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.