As of Drupal 8 all JS needs to follow our JavaScript coding standards. For this we use ESLint to check most of our standards.
Here is the summary of running ESLint on this module JS:
✖ 81 problems (79 errors, 2 warnings)

Attached is the full list of errors found by ESLint and the patch that fixes them. There are a few remaining but not sure how to handle them. Check remaining-eslint-errors.txt

CommentFileSizeAuthor
#9 ga-eslint-2490326-9.patch6.94 KBnod_
google_analytics-eslint-output.txt6.62 KBrteijeiro
remaining-eslint-errors.txt1.31 KBrteijeiro
fix-eslint-errors.patch5.39 KBrteijeiro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hass’s picture

hass CreditAttribution: hass commented
Status: Needs review » Needs work

ga() is a function, not a variable.

  • hass committed 058af0b on 8.x-2.x 
    Issue #2490326 by hass: Google Analytics ESLint errors

  • hass committed dbfadef on 8.x-2.x 
    Issue #2490326 by hass: ESLint - Expected indentation of 2 characters

  • hass committed 6a16b6b on 8.x-2.x 
    Issue #2490326 by hass: Remove trailing space

  • hass committed 172aaa5 on 8.x-2.x 
    Issue #2490326 by hass: ESLint - Extra space after key "@items",
key-...

  • hass committed 6ea4647 on 8.x-2.x 
    Issue #2490326 by hass: ESLint - There should be no space after '{',...

  • hass committed 7a21a98 on 8.x-2.x 
    Issues #2490326 by hass: ESLint - Trailing spaces not allowed,
no-...
hass’s picture

hass CreditAttribution: hass commented
Status: Needs work » Fixed

I have no idea how I can add ga to globals of core .eslintrc file and how I can ignore the console warnings in the debug.js file.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Status: Fixed » Needs review
FileSize
ga-eslint-2490326-9.patch6.94 KB

Here is the patch fixing the rest of the errors.

ESLint config can be overriden within directories, to add the "ga" variable you can declare an eslint config file adding that variable to globals, see the patch.

hass’s picture

hass CreditAttribution: hass commented
Status: Needs review » Fixed

Many thanks for your help and patch.

  • hass committed fc1adde on 8.x-2.x 
    Issue #2490326 by nod_: Extend globals and other bugfixes

The last submitted patch, fix-eslint-errors.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 9: ga-eslint-2490326-9.patch, failed testing.

hass’s picture

hass CreditAttribution: hass commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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

hass’s picture

hass CreditAttribution: hass commented
Status: Closed (fixed) » Active

This has broken nearly all val()'s in vertical tabs.

Broken:

var $radio = $('input[name="google_analytics_visibility_pages"]:checked', context);
if ($radio.val() === 0) {

Should we quote the integer or parseInt()?

var $radio = $('input[name="google_analytics_visibility_pages"]:checked', context);
if ($radio.val() === '0') {

var $radio = $('input[name="google_analytics_visibility_pages"]:checked', context);
if (parseint($radio.val()) === 0) {
droplet’s picture

droplet CreditAttribution: droplet commented

input value is String

hass’s picture

hass CreditAttribution: hass commented
Status: Active » Fixed

  • hass committed c282bbe on 8.x-2.x 
    Issue #2490326 by hass: vertical_tabs broken by previous ESLint changes

Status: Fixed » Closed (fixed)

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