Use a JS module system for JS rules

[jpg0]

Is your feature request related to a problem? Please describe.
I find the custom library loading code frustrating because:

• It's pretty long boilerplate to import a library
• Imports pollute the global namespace
• Writing libraries or importing them is unfamiliar to JS coders
• Not compatible with existing libraries

Describe the solution you'd like

• CommonJS or AMD implementation for libraries

Describe alternatives you've considered

• I have NOT looked into the aboves' compatibility with OH, but assumed that it's possible one way or another :)

[5iver]

I'm pretty certain these will not be possible, as we need to stay native Nashorn for javax.script. Even if it was possible, I do not feel it would be worthwhile implementing a module system, since JDK9/ES6 is very close and will change everything.

[jpg0]

So I was thinking of just something basic which meant that you could use a loader syntax. To see if it was possible, I just created something basic so that you can write code like this:

In script:

load(Java.type("java.lang.System").getenv("OPENHAB_CONF")+'/automation/lib/javascript/personal/loader.js');

var foo = require('foo');
foo.bar();

In library:

exports.bar = function(){ ... }

I can create a PR to add the loader.js file if you like / are interested?
ps.I am also hoping for JDK9 but from the actual movement I see in OH I'm not confident it'll happen for a while.

[5iver]

Here are the outstanding issues with JDK11... it's close! And IIRC, there are less with JDK9.

If you are wanting to actively develop the JS helper libraries, I definitely do not want to hold you up! I'm not much of a JS developer, so hopefully we can get some feedback from @lewie or @Confectrician. I see the benefit of what you are proposing, but my gut is telling me to wait for ES6, or we will have a lot of rework.

[Martin-Stangl]

@jpg0, I have a question for clarification: Is it correct to assume that the library file containing the code exports.bar = function(){ ... } is called foo.js? And is it also located in /automation/lib/javascript/personal/?
Or is there some magic going on in loader.js that maps foo somehow to something?

In general using modules is a great idea. But it would be a major rewrite and keeping backwards compatibility to the current libraries will be quite some effort - probably more than it is worth..

[jpg0]

@openhab-5iver I'm glad that you're more optimistic than I am! It was exactly that issue (and the lack of progress on it + it's dependencies) that made me think it will be a while!

@Martin-Stangl yes that is all correct. Loader is not magic at all, I just made some minor modifications to an existing pure-JS one: https://gist.github.com/bspaulding/1386829. Saying that, it does not currently prevent libraries polluting the global namespace, but well-behaved ones should work fine (this may be possible using loadWithNewGlobal in Nashorn, but I haven't looked into it).

The reason that I am suggesting doing this now is that:

1. It's going to have to happen at some time (to provide a decent JS env)
2. It's only going to get harder - as changing the libs/syntax is the bit that is the most painful, and more code using the existing helper libs is written all the time
3. Selfishly, I'm planning to switch over this for my scripts now, regardless of what happens here :)

As for backwards compatibility, I agree that this is the hardest part. But it will only get harder. There are also quite a few options. For example, maybe the cleanest would be to deprecate the 'core' directory and create another, such as 'common' or 'modules' or something, then just leave stubs in core which 'require' their associated modules, and hoist the exports up into the global namespace. This means that all work can proceed with no ongoing work for backward compatibility and the new work inherits none of the legacy.
Also remember that there's no technical reason preventing both approaches being used concurrently, even in the same file. (Just not for the same lib.)

I guess that one option is for me to do this in a fork (including porting existing libs + maintaining backwards compatibility) and you can see if you're interested in pulling it in - as I want to do it for my scripts anyway - but if this is a something that is very unlikely to be adopted then I wouldn't bother with the compatibility layer if it's only for my scripts anyway.

[jpg0]

Quick update: I've ported most of the existing libs over to a CommonJS style. I decided to just replicate them into a /modules folder and leave the existing ones there: jpg0@e246da5

What I have learnt:

• it all appears to work fine, using standard JS module requires/exports syntax
• referencing things from modules kinda shows up some of the 'kitchen sink' approach of the utils file - for example utils.sendCommand, or utils.getAction etc - all works but highlights that it should probably be broken up.
• the 'scriptExtension.importPreset' commands just dump everything into the global namespace. I haven't tried to do anything about this.
• the nasty 'me' ReferenceError: "me" is not defined in triggers.js #265 issue needs a proper fix and was the only thing that wasn't (trivial, tbh) to port (I just removed the name for now).

[5iver]

I will provide more detailed feedback when I review the PR, but wanted to respond with some of the larger issues as soon as I could.

• Please submit a PR with your changes. It will be much easier to diff the files and view the changes that you have made,
• Why add a modules directory? The core JS libraries are already in /automation/lib/javascript/core/, so I don't see why the core directory couldn't be used instead. This also keeps the directories aligned with the Python libs.
• The core.logutil lib will need to be renamed to match the Python core.log lib. In the future though, this lib (Python and JS) will be replaced with an OH core action or actions, similar to logInfo, logDebug, etc.
• Remove test.js.
• Please include before and after usage examples to show how the libraries would need to be used after this change.
• For symmetry with the Python libs, WDYT of renaming loader.js to init.js?

the 'scriptExtension.importPreset' commands just dump everything into the global namespace.

This is by design. What are you thinking should change?

[jpg0]

Please submit a PR with your changes. It will be much easier to diff the files and view the changes that you have made,

Sure! The reason that I did this was to give very high-level feedback on the approach, as I don't regard this as PR-quality yet. So thanks!

Why add a modules directory? The core JS libraries are already in /automation/lib/javascript/core/, so I don't see why the core directory couldn't be used instead. This also keeps the directories aligned with the Python libs.

The challenge here is backwards compatibility. There is currently no abstraction in the loading of libraries; consumers are encouraged to literally construct the file path and directly load/inline the library code. This leaves a few options:

• create two versions of each library (new to be loaded as a module, old to be loaded directly). This was my approach in creating a new folder. Of course we could reuse the /core folder, but rename the new libs to something like .module.js, but that breaks the CommonJS expectations (which would be that they are imported as require('<lib>.module')). We can't rename or move the old versions because the existing approach is load them directly (if there was an abstraction we could have diverted it for the existing style).
• attempt to do some magic within the existing libraries. This is possible, to have them detect the style in which they are being loaded and react by either exporting functions, or applying them to the global namespace. Possibly a downside to this approach is complexity in libs code (as new libs are not free of the existing approach), but it may not be a big deal. I think it would work though; happy to take this approach if you prefer it.

The core.logutil lib will need to be renamed to match the Python core.log lib. In the future though, this lib (Python and JS) will be replaced with an OH core action or actions, similar to logInfo, logDebug, etc.

Sure, no problem.

Remove test.js.

Sure, as I mentioned this isn't yet PR-quality.

Please include before and after usage examples to show how the libraries would need to be used after this change.

Where would you like the usage examples? Maybe port the example scripts as part of the PR? Ultimately they are standard CommonJS, so the change isn't large, just that to use a function bar in module foo, e.g. it changes from:

load('foo');
bar();

to

var foo = require('foo');
foo.bar();

For symmetry with the Python libs, WDYT of renaming loader.js to init.js?

Sure, no problem.

the 'scriptExtension.importPreset' commands just dump everything into the global namespace.

This is by design. What are you thinking should change?

I was thinking that instead of inlining the code when calling that, that it would return an object which contained the exported functions/variables, just like CommonJS. I do appreciate however that there are cases where consumers would want everything inlined to cut down on characters in simple scripts, so this should remain an option (which is possible if returning an object, merging the object into global level, but picking it back out isn't).

ps. Won't get to this for a few days as I'm away, but I'm actively working on it as I port my scripts from Xtend to JS so I'll be back on it then.

[Martin-Stangl]

@jpg0 just a heads up/warning: I made a pull request #268 for a core/log.js. So this part might look significantly different to your core/logutil.js library soon.
Well probably not significantly for what you are doing as it should convert as easy into a module as the other libraries.

[jpg0]

Added PR: #270

I think I addressed all your points @openhab-5iver, but happy to make further changes. I also managed to implement isolation to prevent libraries from adding to the global namespace.

@Martin-Stangl no worries about #268 - I ported it immediately and am now using it myself.

[jpg0]

Forget that PR, I've updated to #271

Providing isolation between libraries was getting too complex, and was then starting to fight with the importPreset approach which is unisolated by design. I've reverted to a simpler design which forgoes isolation.

[jpg0]

I'll close this issue, as it's now very out-of-date as all this has been solved and more.

I now have:

• a fully CommonJS/nodeJS-compliant module system working on GraalJS (so ES9+)
• baked into the JS host plugin, in Java not JS
• also wraps the importPreset stuff into modules, imported as const { automationManager, foo, bar } = require('@runtime/RulesSupport')
• allows simple installation of 3rd party dependencies via npm install <package>

https://github.com/jpg0/openhab2-addons/tree/master/bundles/org.openhab.automation.module.script.graaljs