openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

Use `function` instead of `const = function` #262

Closed florian-h05 closed 1 year ago

florian-h05 commented 1 year ago

... and enable ESLint rule to enforce this code style. Additionaly, use arrow functions for callbacks.

This has some benefits:

digitaldan commented 1 year ago

Hmm, yeah i have flipped flopped between the two styles, mainly b/c some code base tended to use one or the other. I tend to agree i like declaring functions with 'function', if for no other reason then it looks nicer to me ;-)

Quick question on prefixing private functions with _ since i see that a lot in the diffs (not related to your changes) , its something i have always though of as hacky, especially since there's no enforcement of it in the language, its just a pattern that has emerged over time. While i have actually never seen a code base use it, ES 2019 i believe introduced actual private functions by prefixing them with '#', i would assume grallvm supports this? Might be a option for us?

In any case since there a lot of changes in the PR, i just did a quick look over and trust its been well tested.

florian-h05 commented 1 year ago

@digitaldan FYI I just rebased this PR and enabled the ESLint rule to enforce this code style.

While i have actually never seen a code base use it, ES 2019 i believe introduced actual private functions by prefixing them with '#', i would assume grallvm supports this? Might be a option for us?

AFAIK the # is only available for private fiels and methods of classes, not function declarations. I've tested this syntax some time ago on openHAB 3.4.x and GraalJS in ECMA2021 mode there does not support this syntax. Note that the functions declared are private if they are not exported with module.exports, the _ is just a naming pattern. In openhab-js, no _private functions are exported except when unit tests need them.

In any case since there a lot of changes in the PR, i just did a quick look over and trust its been well tested.

It's actually not that much changes (most changes probably come from the automatically regenerated type defs), but I am testing this in my production setup (as with nearly any PR).

digitaldan commented 1 year ago

FAIK the # is only available for private fiels and methods of classes

Right 🤦