Problem/Motivation

Address standards in these js files

js/module_filter.modules_uninstall.js
js/module_filter.permissions.js
js/module_filter.update_status.js

Steps to reproduce

NA

Proposed resolution

Address issues or document why we can't

Remaining tasks

Implement
Test each file
Review

User interface changes

NA

API changes

NA

Data model changes

NA

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

smustgrave created an issue. See original summary.

jonathan1055 made their first commit to this issue’s fork.

jonathan1055’s picture

Status: Active » Needs review

Just made some changes to get things started. Problem is I can't run eslint locally. So there will be plenty of things still wrong with this, plus I've only tackled the easy bits first.

jonathan1055’s picture

First run took the coding standards for these three files down to 49 (27 fewer than branch)

jonathan1055’s picture

Second run - 31 messages (41 fewer than branch)

jonathan1055’s picture

Down to 27 messages (49 fewer than branch)

jonathan1055’s picture

Two files clean. Just three messages remaining in module_filter.permissions.js

64	for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array. (no-restricted-syntax)
67	for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array. (no-restricted-syntax)
67	The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype. (guard-for-in)
smustgrave’s picture

Can convert them or just to silence the errors. Dealers choice.

jonathan1055’s picture

I'm going to pick this up again and see what the current state is. From the branch pipeline eslint we have

js/jquery.winnow.js
js/module_filter.js
js/module_filter.modules_uninstall.js
js/module_filter.permissions.js
js/module_filter.update_status.js

200 problems (197 errors, 3 warnings)

In the MR26 we have the same set of five files but the log says
149 problems (149 errors, 0 warnings)

Most of these are prettier formatting warnings, which will be good to fix, but we need to separate them out on a separate issue. So I'm going to set them to be ignored in this MR, to see what is left over.

jonathan1055’s picture

That removed all but 1 of the warnings. We now get just

/builds/issue/module_filter-3338336/web/modules/custom/module_filter-3338336/js/jquery.winnow.js
  54:36  error  Unnecessary escape character: \:  no-useless-escape
✖ 1 problem (1 error, 0 warnings)

So this is a new warning, because jquery.winnow.js was clean back in May when you fixed #3338338: Javascript coding standards (winnow.js and module_filter.js). But we can fix that here.

We also do not get the three remaining warning I listed in #8 above. Did your recent work alter/remove some the rules?

jonathan1055’s picture

That fixed it. The pipeline with *.js in .prettierignore runs green for eslint. So this file can be put back (so .js are not ignored) and then this MR can be merged.

I will open a separate issue to look at the prettier warnings. It would be nice to have the branch test passing green.

jonathan1055’s picture

Status: Needs review » Reviewed & tested by the community

smustgrave’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for keeping this one moving!

jonathan1055’s picture

and thanks for merging.

#3405485: Fix prettier formating warnings in javascript files will now be easier to work on, too.

Status: Fixed » Closed (fixed)

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