-
-
Notifications
You must be signed in to change notification settings - Fork 701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JSR223] Mention difference between NashornJS and GraalJS #1846
Conversation
Fixes openhab/openhab-js#139. Signed-off-by: Florian Hotze <[email protected]>
✅ Thanks for your pull request to the openHAB documentation! The result can be previewed at the URL below (this comment and the preview will be updated if you add more commits).
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. I added a few comments based on the current state of Nashorn.
Over all, I think our whole approach to the rules documentation needs an overhaul but I think we should wait to see if Rules DSL gets spun out into a separately installable add-on.
Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
f4c7dde
to
e406f8e
Compare
Signed-off-by: Florian Hotze <[email protected]>
e406f8e
to
4e72292
Compare
@rkoshak |
The text looks great! I'm ambivalent about the JS Scripting example. On the one hand it's useful and clearly illustrates the differences between them. And there are examples for all the other languages there already (except Rules DSL for obvious reasons). On the other hand, JS Scripting isn't actually JSR223 so there is a strong argument that an example like this doesn't even belong here, especially since just about everything that is discussed on this page is hidden behind the JS Scripting Helper Library which is documented elsewhere. Were the example to only use the JSR223 interface as it's discussed on this page, it would look almost exactly the same as the Nashorn example with the addition of a Given that the topic of this page is the JSR223 runtime interface, any JS Scripting example should probably use that interface directly, perhaps with a note that using the Helper Library results in a much simpler and straight forward implementation (which is also true for jRuby which probably should have a similar note ). |
Reagrding JS Scripting: |
That would be a minimum but I'm still a little concerned. jRuby could do the same thing. But still one has to ask what's the point of the example if it's not demonstrating the APIs that the page is documenting? This page documents the Java API but the helper library almost complete wraps/replaces that API. And the Helper Library is fully documented elsewhere so if I wanted to look on this page for more information about JSRule, triggers, etc. I wouldn't find any of that here on this page. It's confusing to have an example that doesn't actually demonstrate the use of anything that's documented on the page is my main point I think. I can think of ways to make that make sense but it'll require almost a complete restructuring of this and the Rules documentation pages or the introduction of another "generic rules concepts" page. |
Signed-off-by: Florian Hotze <[email protected]>
291de9e
to
e9e6445
Compare
I updated the JS Scripting example to have one using the raw Java APIs and to have the same using the helper library (with a comment that clearly indicates that it is using the helper library). However, I was not able to get the example for the raw APIs working with Just look at the docs preview. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from docs side.
Please give me a ping, when it is ready for review/merge from the content side too.
So this didn't work? Or did you try to import some other way? I've not done file based rules with JS (either version) so I don't know if there is something different. But when I was looking in how to make Nashorn compatible with JS Scripting, the easiest thing was to pull in
If that or something like that won't work, I'm good with the example as it is. Even if that does work, I'm OK with the example as it's currently written so I leave it up to you if you want to change it to be consistent with the other examples (there is some value in that) or keep it different and therefore showing an alternative way to do it. |
I also thought it would work that way (I was using |
I think that wouldn't be sufficient because I think the presets get imported to the |
Sorry i'm late to the party here, but i wonder if we want to keep this page about "JSR223", or rather just make it "Scripting" and add in the rules DSL to it. As a user i would think its super confusing why its spread around like this across multiple pages, and at the end of the day do they really care about the underlying Java extension , or do they care about the rule languages (Javascript, ruby, DSL, etc...) themselves? |
Ultimately I think we do need some page to document the actual Java openHAB rules API because there will always be something that a helper library doesn't do or the way it works needs to be worked around for some reason. But it should become an advanced topic and not something that most users ever need to read since they are not using this API directly but instead using the various helper libraries. However, once we start going down that path we would ultimately end up with a complete reworking of our entire approach for rules documentation in general. I've actually been thinking about this for some weeks now but lack the time to really work on it much. But were I to start on something like this I'd use the following structure:
We need to extract the generic stuff from the current Rules page and move the Rules DSL stuff to its own page. We need to push people to use their respective helper libraries instead of the RAW API. Over all, the Rules docs need to look more like the UI section in the docs.
Several of the languages including Groovy and JRules only work through the raw API. Other languages have a third party helper library (i.e. not part of the openHAB project itself) including Jython, Nashorn, and jRuby and users may not want to or be able to install these and instead would rather use the language without the helper library. And not all the helper libraries do as good of a job abstracting the raw API. Anyway, for this specific PR though, since we are not looking to do a complete reworking of the rules docs and not reworking this specific page, I strongly believe that if we include a JS Scripting example at all, it should use the raw API instead of the helper library. Otherwise it's kind of a pointless example given what this page is about. When, if the rules docs do get reworked, then there will be lots of other places to include "proper" examples using openhab-js, not to mention the examples in the JS Scripting add-on itself. |
Object.assign(this, require('@runtime'));
Object.assign(this, require('@runtime/RuleSimple'));
Object.assign(this, require('@runtime/RuleSupport')); … imports everything needed for the creation of a rule, but I get the following error:
So const TriggerBuilder = Java.extend(Java.type('org.openhab.core.automation.util.TriggerBuilder')); everything is working. FYI: Using I updated the example to use less |
Signed-off-by: Florian Hotze <[email protected]>
@rkoshak And can you please have a look at my current example? If it‘s good, we are ready to merge. |
Looks good to me. Thanks for doing this. @Confectrician this looks ready to merge Issue to rework the rules docs is #1855 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. 👍🏼
Fixes openhab/openhab-js#139.
This explains the difference between the two available JavaScript options and recommends to use JS Scripting.
Signed-off-by: Florian Hotze [email protected]