Style error information

https://eslint.org/docs/rules/no-useless-escape

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
yarn & yarn lint:core-js-stats

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

dawehner’s picture

Here is some patch

GrandmaGlassesRopeMan’s picture

@dawehner Is the transpiled JS missing from this patch?

dawehner’s picture

oops, I forgot the transpiling.

GrandmaGlassesRopeMan’s picture

Status: Needs review » Reviewed & tested by the community

I think this looks alright. Some of these were holdovers before template strings. 👍

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2916294-3.patch, failed testing. View results

dawehner’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
8.7 KB

Falling back to patching base and 3-way merge...🎆

Here is a quick reroll.

  • xjm committed 7fbb5e9 on 8.5.x
    Issue #2916294 by dawehner: JS codestyle: no-useless-escape
    
xjm’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
FileSize
460.88 KB

This issue has been an exciting test of my regex skills, both in terms of when escaping is and isn't needed for special characters, and in crafting a word diff to diff them. (As well as a test of my eyesight.)

Turns out the word diff is actually super simple. Let's call it a "character diff", even:
git diff --staged --color-words="."

The only changes are removed \ in places that https://eslint.org/docs/rules/no-useless-escape says they can be removed safely. The aforementioned page helped reassure me that it is in fact safe to not use \ on [ inside a character class. I also learned something. I did not expect the following to work, but the page reassures me that it will not result in a behavior change:

In PHP it would have been, just, like, wrong (in HEAD that is).

I read over the changes carefully and linted before and after. Before:

Errors:
==============================
no-use-before-define: 75
max-len: 70
camelcase: 65
no-restricted-syntax: 54
no-shadow: 42
no-useless-escape: 19
new-cap: 18
default-case: 16
no-continue: 9
no-new: 9
no-throw-literal: 1

After:

Errors:
==============================
no-use-before-define: 75
max-len: 70
camelcase: 65
no-restricted-syntax: 54
no-shadow: 42
new-cap: 18
default-case: 16
no-continue: 9
no-new: 9
no-throw-literal: 1

So, committed and pushed to 8.5.x. Thanks!

dawehner’s picture

Turns out the word diff is actually super simple. Let's call it a "character diff", even:
git diff --staged --color-words="."

That's super cool. Thank you for sharing this! Now I jut need to remember it by thinking about no useless escapes.

I did not expect the following to work, but the page reassures me that it will not result in a behavior change:

Me neither to be honest, I'm glad eslint helps us to catch these differences between languages.

Status: Fixed » Closed (fixed)

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