Style error information

https://eslint.org/docs/rules/vars-on-top

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

CommentFileSizeAuthor
#2 2915766-2.patch337 bytesdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Fixed » Needs review
FileSize
2915766-2.patch337 bytes

Here is my little patch. Now that we dropped vars, we no longer want to put them on the top anyway :P

GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Issue summary: View changes
GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Status: Needs review » Reviewed & tested by the community
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed 
The Hall of JavaScript
> LINT
What do you want to lint?
> LINT THE JS
You cannot lint the JS.
> ASK MATT FOR HELP.
Matt says to try "run".
> RUN LINT
What do you want to lint?
> RUN LINT THE JS
You cannot lint the JS.
> ASK MATT FOR HELP.
Matt asks you if this is 8.5.
> GIT CHECKOUT 8.5.x
You switch to 8.5.x.
> LINT THE JS.
You linted the JS. There are no new errors.

I reviewed this as follows:

  1. Ran yarn lint:core-js-stats against 8.5.x HEAD and saved the output.
  2. Applied the patch and confirmed there was no change to the above output (other than the runtime).
  3. Hacked in this:
    diff --git a/core/misc/drupal.es6.js b/core/misc/drupal.es6.js
index e64f9dac0d..fe19c839b5 100644
--- a/core/misc/drupal.es6.js
+++ b/core/misc/drupal.es6.js
@@ -56,6 +56,7 @@ window.Drupal = { behaviors: {}, locale: {} };
     setTimeout(() => {
       throw error;
     }, 0);
+    var foo;
   };
  4. Confirmed that the rule would fail with that regression introduced:
    diff --git a/core/tmp b/core/tmp
index eccc5aff29..adb09197c3 100644
--- a/core/tmp
+++ b/core/tmp
@@ -12,9 +12,11 @@ no-useless-escape: 19
 new-cap: 18
 default-case: 16
 no-multi-assign: 14
-no-new: 9
 no-continue: 9
+no-new: 9
 prefer-rest-params: 3
+no-var: 1
+vars-on-top: 1
 no-empty: 1
 no-throw-literal: 1
 
@@ -26,6 +28,6 @@ Warnings:
 ==============================
 func-names: 468
 no-mutable-exports: 145
-no-unused-vars: 120
+no-unused-vars: 121
 no-plusplus: 16
-Done in 6.24s.
+Done in 6.45s.

Adding issue credit for @drpal for helping me get it working (not for the lack-o-review above). ;)

Committed and pushed to 8.5.x. Thanks!

  • xjm committed a28fb18 on 8.5.x 
    Issue #2915766 by dawehner, drpal: JS codestyle: vars-on-top
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

finally you put my vars on top

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented

😂😹

Status: Fixed » Closed (fixed)

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