[jsscripting] Allow loading files relative from root folder automation/js

[csowada]

I miss the option to load relative files within the automation/js folder. So I can't separate my small helper functions from my rules without own modules in node_modules folder.

I've used a custom version of the automation in the past. But I've changed the path for js.commonjs-require-cwd to the root folder instead of the node_modules folder. This allows us to load files from the folder automation/js and node_modules. So it is not required to create own node modules in the node_modules folder.

I've already build custom snapshot and it works as expected but I'm not sure if there is more to check. I only changed two lines to change the root directory for CommonJS loading.

But we have still one disadvantage, the FileWatcher will load all files in the folder except the node_modules folder. So the config.js in my example would also be loaded on startup. But in that case I just want to load the file via a require call in my rule. So we still need a solution not only for the node_module folder. My suggestion is to provide an optional file similar to a .gitignore file to skip all folder and files defined in that file.

I think it's more a minor bug in the current implementation instead of a feature, but this is a question for @jpg0 and @digitaldan . If you agree with me I could create a PR.

In OpenhabGraalJSScriptEngine.java

.allowHostAccess(hostAccess).option("js.commonjs-require-cwd", JSDependencyTracker.LIB_PATH2)

and in JSDependencyTracker.java

public static final String LIB_PATH2 = String.join(File.separator, OpenHAB.getConfigFolder(), "automation", "js");

<folder> automation
  -<folder> js
    - myrule.js
    - <folder>config 
      - myconfig.js
   - <folder> node_modules
     - <folder> mylib
       - ... 

const mylib= require("mylib");
const config = require("./config/myconfig");

...

Your Environment

• Version used: openHAB 3.2 release

[jpg0]

Looking at the doc for GraalJS, it states that you can use either the root or node_modules folder for js.commonjs-require-cwd. I agree with you that it probably makes more sense to use the root.

As for the subsequent issues with loading non-libraries, it seems that we have a number of choices:

• Implement some mechanism to ignore non-scripts - such as .ignore file
• Implement some mechanism to point to the scripts to load (the inverse of the above)
• Implement some mechanism to specify library folders (and don't support modules not in a folder)
• Don't load scripts in subdirectories at all, or modules in the root
• Implement a toggle to either 'assume subdirectories contain scripts' or 'assume subdirectories include libraries', and don't support modules in the root folder
• Don't support modules not in node_modules at all (so relative scripts work fine between modules, but not from scripts to modules)

We should ensure that we make the simplest use-cases the most simple, so IMO this rules out (2), even though it probably most closely matches the standard Node behaviour (e.g. package.json). However, we could tweak this option to say that all scripts are loaded unless a package.json is found, in which case it points to the list of scripts. This would track the closest to the standard, also allowing all requirements including modules in the root folder.

Do you think that many people will want both the behaviour of some modules in a relative subdirectory AND some scripts in subdirectories? What about modules in the root? These are all supported by standard JS/npm. This is what I am struggling with; it seems like a pretty rare use-case so I'm loath to try to support it (rather than make a decision on one or the other).

@JamieTemple you wanted the ability to load scripts from subdirectories. Would you also want relative modules?

[csowada]

A short answer to you six points and my personal ratings:

Implement some mechanism to ignore non-scripts - such as .ignore file

👍👍 This is the most flexible solution, but the filter is more complex to implement. But it would not change anything for the users if it is not used.

Implement some mechanism to point to the scripts to load (the inverse of the above)

👎 Mh, you thinking about something like a index.js file. I think this would make the FileWatcher more complex? I would not go this way as it would change the current behavior too much.

Implement some mechanism to specify library folders (and don't support modules not in a folder)

👍 I would guess a small list of reserved folder names to ignore would be enough. (.git, node_modules, libs)

Don't load scripts in subdirectories at all, or modules in the root

👍 Simple solution, put you rule in the root folder and all helper files in a subdirectory.

Don't support modules not in node_modules at all (so relative scripts work fine between modules, but not from scripts to modules)

👎👎 So you mean we should not change the current implementation. In that case you still have the issue with files in the modules directory. You must build your own modules to just have central functions in your rule. It is not easy for non npm developers to understand.

---

However, we could tweak this option to say that all scripts are loaded unless a package.json is found, in which case it points to the list of scripts. This would track the closest to the standard, also allowing all requirements including modules in the root folder.

I agree with you, this could be a valid option, but I think this is not a simple solution or? Or is that working out-of-the box with the GraalVM CommonJS implementation?

Do you think that many people will want both the behaviour of some modules in a relative subdirectory AND some scripts in subdirectories? What about modules in the root? These are all supported by standard JS/npm. This is what I am struggling with; it seems like a pretty rare use-case so I'm loath to try to support it (rather than make a decision on one or the other).

I don't really follow. In CommonJS, practically every file is a module that I can load. I would like to load relative modules in relation to the root directory and furthermore ready-made libraries with absolute names (i.e. without ./ or ../). In this case, I would consider it a not an unusual case.

Or did you mean something else?

[jpg0]

I agree with you, this could be a valid option, but I think this is not a simple solution or? Or is that working out-of-the box with the GraalVM CommonJS implementation?

No this would need to be built. But it's no more complex that your suggesting of a .ignore file (which is also no more complex than my second option that you were against due to complexity!).

Or did you mean something else?

I'm not sure if you're aware of this, but:

• JS allows loading modules in the root folder too, e.g. require('./foo'). This conflicts with the current "load all files in the root as scripts" behaviour of openHAB
• We already have openHAB users who put JS files in subdirectories to organise their scripts, expected them to all be loaded as scripts. Again, more conflict.

This means that some of your explanations are incorrect, such as "I would guess a small list of reserved folder names to ignore would be enough.". It also means that option (4), whilst solving your problem, breaks functionality for others.

[csowada]

Thank you for your answer. I just wanted to comment on your points and answer them like a survey.

Of course, each point has its advantages and disadvantages. I should also not evaluate what is complex and what is not, that depends a lot on the solution. That was just a gut feeling from my side.

But just for my understanding, by scripts you mean JavaScript files that are executed directly on refresh or startup. And with library directory you mean directory incl. JavaScript files (modules) that are loaded via require/import.

So if we implement this feature and do not want to interfere with the existing user rules, the following options would remain:

• (1) Implement some mechanism to ignore non-scripts - such as .ignore file
• (2) Implement some mechanism to point to the scripts to load (the inverse of the above) (but needs a fallback?)
• (3) Implement some mechanism to specify library folders (and don't support modules not in a folder)
• (5) Implement a toggle to either 'assume subdirectories contain scripts' or 'assume subdirectories include libraries', and don't support modules in the root folder

All other options would change the current behavior or ? For me all options are an enhancement.

@jpg0 If you agree I would start with a first try or do you have already other plans?

[csowada]

I experimented a bit. Here is a demo for loading from the js root directory including the glob filters like .gitignore. We should think about whether this change should be included in the ScriptWatcher of openHAB core.

But this is just a small demo to estimate the effort! It is not a complete solution.

https://github.com/csowada/openhab2-addons/pull/11

[csowada]

Okay, now my pull request is finish for a first try. Feel free to try the build version.

https://github.com/csowada/openhab2-addons/releases/download/jsscripting-v2-1/org.openhab.automation.jsscripting-3.3.0-SNAPSHOT.jar

[openhab-bot]

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/jsscripting-in-v3-2-loads-node-modules-on-startup/130357/6

[jpg0]

@jpg0 If you agree I would start with a first try or do you have already other plans?

You can start, however I think that it's good to decide on an approach first. Personally, I'm not a fan of option (1) above. My belief is that it's too complex - well, it's complexity for the extreme end of the scale - that the user:

• Is using custom/personal modules
• Wants to use them in a relative directory (not node_modules)
• Wants support for them to be in the root and/or also wants support for scripts within subdirectories

This is a vanishingly small edge case. However using an ignore file will not be trivial - for users to understand how to use, for them to debug the globs and for maintainers to build and maintain. One aspect of it's complexity would be the interaction between JS file (and directory) changes with changes in the ignore file. For example, in your demo code, if I update the globs to add or remove existing files, they will not be added to or removed from the script engine. Of course this can be added, but it's more complexity (especially if you want to ensure timing/ordering works robustly).

I remember a colleague that I worked with saying "every configuration option is from a PM (product manager/owner) who couldn't make a decision". Whilst I think that's a little extreme, I think that this is a situation where is shows merit.

I think that if I were to make this decision now, I would opt to go as close to Node as possible, so to use a package.json to provide extra information. I would:

• Only load scripts that were specified in package.json, under an openHAB-specific key, such as oh-scripts. I would allow globbing in this, as well as standard file names etc.
• If this key is missing or has no value, I would log a warning explaining what was happening (in case someone accidentally ends up with this file here).
• If there was no package.json detected, then I would fallback internally to an internal, predefined one, which would most likely specify everything (inc. subdirs, but node_modules would also be excluded regardless) as scripts.

The reason that I like this option is that it effectively offers the same functionality as an ignore file, yet is inverted (choose the scripts you want, rather than the libraries that you don't want as scripts). It also much more closely matches Node, and I've actually found it really useful to include a package.json in your scripts, because you can define your dependencies and then install or update them all with npm install, or use any of the other npm functionality. This would also work with existing JS tooling (well, it would include the package file), and also provide a place to put future configuration for scripting should we need it.

@digitaldan do you have an opinion on this area?

[csowada]

For example, in your demo code, if I update the globs to add or remove existing files, they will not be added to or removed from the script engine. Of course this can be added, but it's more complexity (especially if you want to ensure timing/ordering works robustly).

Yes you are right, there is no update mechanism. We would miss a good solution if we would go further.

In the end I'm happy with both solutions. It should be easy to understand, be compatible with oh3.1/3.2 implementation and should not be too complex to maintain.

I've tried to illustrate you idea in an example folder structure. Is my example what you mean? After thinking about it for a while for the example, the approach is quite interesting. But I think we will have the same problem with refreshing all relevant scripts after the change of a package.json file or?

• root
  • my-script-at-root.js (run at startup and update)
  • mymodule folder
  • package.json (without specific key or empty)
  • do-not-load-me.js (do no load as script)
  • myconfiguration.js (do no load as script)
  • myscript folder
  • package.json (contains specific key with "my-script-in-sub.js" but not "my-module-in-sub.js")
  • my-script-in-sub.js (run at startup and update)
  • my-module-in-sub.js (do no load as script)
  • plain-old-script folder without package.json file (compatibility mode - load all as scripts)
  • load-as-script.js (run at startup and update)

[jpg0]

This isn't quite what I meant, as I was assuming a package.json in the root folder, which would point at the scripts to load. So in your example, either the root package.json would have:

• myscript/my-script-in-sub.js, and the myscript/package.json would be unnecessary
• or myscript, which would inform the root to also process myscript/package.json. Although I was not even expecting that we would support sub package.json's however I guess we should do it we want full relative module support.

The plain-old-script directory would not run in compatibility mode - either everything is, or nothing is, so given that a package.json exists in the root, plain-old-script/load-as-script.js would need to be in the root package.json too.

And yes, this would require handling changes in package.json, just like an ignore file would.

[openhab-bot]

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/timeline-picker-to-setup-heating-light-and-so-on/55564/725