Improve coding style (general)

[brodycj]

Some ideas that I raised in review of PR #764, some of which may apply to other Cordova packages:

• use JavaScript array methods such as forEach, map, and reduce to get rid of for loops
• in case of JavaScript objects, use keys method to get the array of keys then use array methods to get rid of for loops
• use const (first choice) or let (second choice) instead of var
• use ternaries to get rid of if blocks
• use nested ternaries to get rid of multiple if blocks (possibly controversial)
• use utility packages such as recursive-readdir-sync whenever possible instead of shelljs and to reduce the amount of “fs*” code we have to maintain

I am raising this issue before it gets forgotten. Maybe we should transfer or write this issue in https://github.com/apache/cordova/issues for larger discussion and consideration.

[janpio]

Good idea to start a discussion on this, but I agree this can probably be a Cordova level discussion.

Can you please annotate the individual suggestion with the connected Node/ES requirements so the impact of these changes become clearer?

Other discussions about what you suggest can happen when the issue is in the right place and properly framed. Thanks.

[dpogue]

• use ternaries to get rid of if blocks
• use nested ternaries to get rid of multiple if blocks (possibly controversial)

I pretty strongly disagree with these suggestions, especially nested ternaries. Cordova code is already often hard to follow, and ternaries make it even harder to read and understand.

[breautek]

I noticed that the cordova projects use eslint but doesn't take advantage of shareable configs.. At my company we have several repositories where we want to use a consistent eslint configuration. To ensure this, we actually created an eslint config module.

Basically you name the module, eslint-config-<yourconfig>, so in cordova's case... it would be eslint-config-cordova I guess, and you can define all your rules inside this module, which all cordova projects & plugins would include as a dev dependency.

"eslint-config-semistandard": "^13.0.0",
"eslint-config-standard": "^12.0.0",
"eslint-plugin-import": "^2.14.0",
"eslint-plugin-node": "^8.0.1",
"eslint-plugin-promise": "^4.0.1",
"eslint-plugin-standard": "^4.0.0",

These eslint plugins and configs if they apply across all cordova packages can also be dependencies of our custom config module therefore every cordova package doesn't need to install each package manually either.

[janpio]

(Another thing for the more generalized version of this issue that probably didn't get created yet)

[breautek]

@janpio I recently discovered that https://github.com/apache/cordova-discuss is a place to suggest proposals? Would this make sense to do for the eslint config I idea I mentioned above?

[janpio]

IMHO this is not really the case, but some people seem to use it that way, so why not. (The official location would be the dev mailing list with a link to a thought out description either in mail or via link)

[breautek]

I would also strive to add "use strict;" in the javascript sources, and fix any code that would be preventing us from using strict mode.

Strict mode offers 2 main benefits:

1. Enables optimizations in the JS engines to make JS perform faster whereas without strict mode, the JS engines cannot possibly make such assumptions during its interpretations.
2. Helps prevent some JS bad practices being added in the future as most of them will lead to actual errors.

[brodycj]

Any objections if we transfer this to https://github.com/apache/cordova/issues?

I think some of these ideas are covered by apache/cordova#142.

[brodycj]

[...] add "use strict;" [...]

I think we should do this, since we are still not using ES6 modules.

Node.js is in a strange situation where it does support ES6 but not ES6 modules unless we start using an experimental flag along with some other changes as documented in https://nodejs.org/api/esm.html. (This may change in Node.js 12 LTS according to https://2ality.com/2019/04/nodejs-esm-impl.html#using-es-modules-on-node.js.)

Maybe this should be a separate discussion in https://github.com/apache/cordova/issues?

Any objections if we transfer this to https://github.com/apache/cordova/issues?

I tried to do this, GUI does not seem to let me select apache/cordova.

[jcesarmobile]

closing in favor of the linked issue in cordova repository