Lint and Prettify JavaScript files

[meganindya]

I ran a statistics on the entire codebase. See results.

As far as the (JS) code is concerned, there are 103 .js files in the /js directory, which have a total of 82312 lines of code, 10751 blank lines, and 9796 comment lines, making a sum total of 102859 lines in the files.

Here's the per file count:

lines  filename
-----  --------
7325  js\block.js
6839  js\utils\munsell.js
6292  js\blocks.js
5320  js\widgets\phrasemaker.js
5312  js\activity.js
4427  js\utils\musicutils.js
3079  js\widgets\musickeyboard.js
2925  js\widgets\rhythmruler.js
2732  js\widgets\timbre.js
2560  js\logo.js
2279  js\widgets\temperament.js
2128  js\turtle-singer.js
1936  js\blocks\PitchBlocks.js
1867  js\protoblocks.js
1709  js\macros.js
1709  js\utils\synthutils.js
1580  js\blockfactory.js
1473  js\turtle-painter.js
1421  js\palette.js
1365  js\utils\utils.js
1191  js\js-export\constraints.js
1184  js\blocks\WidgetBlocks.js
1120  js\widgets\modewidget.js
1080  js\lilypond.js
1079  js\turtles.js
991   js\blocks\ProgramBlocks.js
933   js\turtle.js
932   js\blocks\EnsembleBlocks.js
890   js\blocks\NumberBlocks.js
883   js\blocks\FlowBlocks.js
871   js\blocks\ActionBlocks.js
868   js\blocks\MeterBlocks.js
848   js\blocks\RhythmBlocks.js
831   js\blocks\IntervalsBlocks.js
828   js\blocks\PenBlocks.js
810   js\rubrics.js
799   js\widgets\pitchdrummatrix.js
798   js\turtleactions\PitchActions.js
771   js\turtledefs.js
748   js\blocks\RhythmBlockPaletteBlocks.js
748   js\js-export\ASTutils.js
744   js\toolbar.js
710   js\blocks\GraphicsBlocks.js
665   js\blocks\VolumeBlocks.js
664   js\blocks\MediaBlocks.js
662   js\blocks\ToneBlocks.js
652   js\artwork.js
640   js\widgets\pitchstaircase.js
617   js\js-export\interface.js
616   js\blocks\SensorsBlocks.js
559   js\widgets\meterwidget.js
550   js\blocks\ExtrasBlocks.js
510   js\widgets\help.js
500   js\SaveInterface.js
487   js\notation.js
480   js\abc.js
447   js\turtleactions\RhythmActions.js
438   js\turtleactions\ToneActions.js
428   js\js-export\export.js
425   js\widgets\widgetWindows.js
418   js\widgets\tempo.js
409   js\blocks\BoxesBlocks.js
399   js\widgets\jseditor.js
384   js\turtleactions\MeterActions.js
375   js\widgets\status.js
367   js\js-export\generate.js
352   js\blocks\HeapBlocks.js
342   js\mxml.js
341   js\blocks\OrnamentBlocks.js
337   js\blocks\BooleanBlocks.js
332   js\pluginsviewer.js
322   js\js-export\samples\sample.js
313   js\blocks\DrumBlocks.js
307   js\utils\mathutils.js
301   js\turtleactions\IntervalsActions.js
268   js\turtleactions\VolumeActions.js
256   js\turtleactions\DictActions.js
252   js\trash.js
214   js\turtleactions\DrumActions.js
211   js\blocks\DictBlocks.js
195   js\utils\platformstyle.js
157   js\widgets\pitchslider.js
142   js\languagebox.js
141   js\pastebox.js
136   js\widgets\oscilloscope.js
123   js\turtleactions\OrnamentActions.js
105   js\js-export\API\PitchBlocksAPI.js
102   js\widgets\statistics.js
99    js\boundary.js
97    js\js-export\API\PenBlocksAPI.js
97    js\sugarizer-compatibility.js
91    js\basicblocks.js
84    js\js-export\API\GraphicsBlocksAPI.js
74    js\js-export\API\ToneBlocksAPI.js
72    js\js-export\API\MeterBlocksAPI.js
71    js\js-export\API\RhythmBlocksAPI.js
56    js\js-export\API\IntervalsBlocksAPI.js
56    js\js-export\API\VolumeBlocksAPI.js
49    js\js-export\API\DictBlocksAPI.js
49    js\js-export\API\DrumBlocksAPI.js
46    js\js-export\API\OrnamentBlocksAPI.js
21    js\background.js
21    js\loader.js

There are 25 files with more than a 1000 lines of code. Evidently, dealing with so many files and these large number of lines is a challenge. Also, code has been written over a period of 6 years — mostly added on top of the existing code at each time. So, there is an insufficiency of standard in place. There exists code stretching ES5 - ES8.

The goal, here, is to maintain a standard which could help towards better code comprehension, handling, and management. I have previously configured GitHub Actions' Super Linter with ESLint configurations. However, that currently is inactive (it ignores all warnings and errors at the moment), because it finds numerous anomalies.

On running ESLint at the time of writing this yields:

✖ 4521 problems (907 errors, 3614 warnings)
  143 errors and 3004 warnings potentially fixable with the `--fix` option.

We need to improve these files gradually. I've recently added a .prettierrc.json configuration file to auto prettify the files. The .eslintrc.json contains current linting configurations.

I personally use Visual Studio Code - Insiders as my text-editor cum IDE with several extensions that help with writing JavaScript. I keep ESLint and Prettier running.

[ricknjacky]

@meganindya I want to work on this, Any heads up on the plan of action? :)

[meganindya]

We need to come to a conclusion first regarding the linting and formatting rules. We have a bigger goal of adding some kind of automated testing at both the code end (e.g. unit tests) and the application end (e.g. crashes). But, I think that needs to wait before other inconsistencies are sorted, this one being one of them.

@walterbender comments?

[walterbender]

Is there a decent JavaScript equivalent of https://pypi.org/project/black ???

[meganindya]

AFAIK Prettier is the most popular code formatter for JavaScript. There's a CLI, and extensions/plugins for modern IDE like text-editors (e.g. VS Code). And, it can work in conjunction with ESLint. I believe we need both — a code formatter for code readability, and a linter for maintaining coding standards.

[walterbender]

We need to tweak the prettier parameters -- I've run it over the code in the past and it made a real mess (from my Pythonic POV).

[meganindya]

The .prettierrc.json file contains the configurations. As such there aren't a lot of options to deal with; most of these are fairly obvious ones. However, it indeed makes tons of changes since the files we have right now aren't formatted.

Which options according to you need changing?

[walterbender]

You've seen a lot of the same issues as me in your code clean ups:

if (
foo &&
bar
) {

is a particularly annoying one.

[meganindya]

If the conditions are long, they'll indeed break up like this into multiple lines, but there is an indentation. Sometimes I've observed that the indentation isn't proper, most often due to using tabs instead of spaces.

Isn't

if (
    foo &&
    bar
) {
    ...
}

the convention if they can't fit within the line character limit?

[walterbender]

I'd prefer:

if (foo &&
    bar) {

But ESlint is probably the first task. It'd be good to break up some of the larger files too. I started work on block.js last week -- moving a bunch of block-specific definitions to the block prototype definitions, e.g., the default pie menu values. I'd like to move all of the pie menu (controller) code out as well. This is going to take some time. (But we are in much better shape than we were a year ago.)

[meganindya]

I think we should totally port to ES6+ as well. logo, turtle, singer, painter, and a few more were ported. The rest remain.

[walterbender]

Maybe we should add a checkbox list of files that need ES6+ porting?

[meganindya]

I'll go through the files and create the checklist in a new issue. I don't think even the var to let thing was completed. This porting will solve a lot of the linting issues (e.g. no-use-before-declare) I think. We can then come back and work on the linting rules thereafter.

[meganindya]

Btw, we also need to change the requireJS style of importing the files. Possibly, we might actually treat most of them as completely independent modules, rather than adding everything to the global scope.

[walterbender]

I finished pulling the pie menus out of block.js. Still lots of opportunity for cleaning things up, but it is at least a bit more manageable.

[meganindya]

I predict that's pretty much shrinking the size of that file in half?

I plan to work on activity.js. All the initial configuration stuff is there, maybe I can make some improvements, while porting it.

[walterbender]

Yes. Almost exactly a 50-50 split.

[walterbender]

Took care of a lot of var --> let clean up today. But still a little more to do:

activity: 16
turtlesinger: 17

[meganindya]

I guess we should use const wherever logical

[ricknjacky]

I'll go through the files and create the checklist in a new issue.

hey @meganindya I await this, so that I too can contribute to this.

[meganindya]

@ricknjacky see #2629. I'll just add the checklists quickly.

[b-thebest]

Could you assign me this task

[abhishekkumar08]

Hey @meganindya can I make a PR to prettify all the codebase, at once, it would not induce any breaking changes, as I'm just formatting the whole code, so it might be easy to review that. What's your opinion?

[meganindya]

Please give a count of the number of files that'll be affected.

[abhishekkumar08]

Please give a count of the number of files that'll be affected.

For the first commit I would be pushing the changes in the js folder for the files.

         js/SaveInterface.js
         js/abc.js
         js/block.js
         js/boundary.js
         js/languagebox.js
         js/lilypond.js
         js/logo.js
         js/macros.js
         js/palette.js
         js/piemenus.js
         js/pluginsviewer.js
         js/protoblocks.js
         js/rubrics.js
         js/sugarizer-compatibility.js
         js/turtle-painter.js
         js/turtle-singer.js
         js/turtle.js
         js/turtledefs.js
         js/turtles.js

That's a total of 19 files.

[meganindya]

Go ahead ... create a PR

[walterbender]

I think the lint errors are sorted out.