Skip to content
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

JS Rewrite #6

Draft
wants to merge 1 commit into
base: ivans-updates
Choose a base branch
from
Draft

JS Rewrite #6

wants to merge 1 commit into from

Conversation

CrazyIvan359
Copy link
Owner

This is an (almost) complete rewrite of the JavaScript/ES5 portion of the libraries to bring them in line with the features available in the Python libraries.

Work in Progress not everything here has been tested, not everything from Python has been ported yet, and in its current state this version is not backwards compatible with the current JS libraries! You have been warned.

To-Do:

  • add OH3.x support
  • fix Generic Triggers
  • merge PersistenceExtensions.js
  • bring actions.js more inline with original/jython
  • merge conditions.js
  • port date.py to JS
  • cleanup and test providers.js
  • restore original rule helper in rules.js (or do we?)
  • restore original triggers in triggers.js (or do we?)
  • restore original functions etc in utils.js (or do we?)
  • final comparison to jython to match features
  • thorough testing

@rkoshak
Copy link
Collaborator

rkoshak commented Feb 16, 2021

I've answered a few questions on this on the forum already. Working with Lists in JavaScript is awkward compared to the other languages. The options, as far as I can tell are to:

  1. Convert the List to a native JavaScript Array and then manipulate it that way. But JavaScript 5 lacks some important functions like filter and findFirst.

  2. Use the native Java Streams API. but this too is awkward because you don't actually get a List out after you are done and instead need to collapse the stream using a Collector.

I wonder if it makes sense to have a separate language specific library file where we can put stuff like what one would need to write to address this awkwardness that only exists in JavaScript. Python's list comprehensions and built in list manipulation functions work just fine.

WDT? It would probably be a part of a separate PR but since you are rewriting the lib anyway maybe it makes sense to add that as part of this.

@CrazyIvan359
Copy link
Owner Author

Have you see the Java forEach method? I believe everything that is subclassed from Collection has that method, you give it a function that takes 1 argument (List, Array, etc) or 2 (Maps, etc) and it runs through all elements and calls that function for each one. The JS I did used that since it seemed like the easiest way, and easier than streams by the sound of it. I can provide an example when I'm not on my phone.

@rkoshak
Copy link
Collaborator

rkoshak commented Feb 17, 2021

That works for forEach. But not for filter, findFirst, map and reduce all of which are operations people are used to doing, particularly with members of a Group.

forEach is available in those Classes that implement the Iterable interface (basically Lists and Sets). If one is just going to iterate over the elements that would for sure be better. But for the rest the only way I've seen to do it without hand coding the loops is to use the streams API.

And the streams API isn't really all bad and I'm sure we can hide much of the oddness (when compared to how it works in Jython and Rules DSL) through some library calls. The big gotcha is the need to import a Collector and using that to convert the stream back into List.

@CrazyIvan359
Copy link
Owner Author

An interesting problem for sure, we will need to do some experiments. I stopped using all of those functions when I left DSL rules, but then again I'm using to programming in CPython where those don't exist so I just do what I know.

@rkoshak
Copy link
Collaborator

rkoshak commented Feb 17, 2021

They exist in Python as well, though they have largely been replaced with list comprehensions as the more Pythonic approach. https://www.learnpython.org/en/Map,_Filter,_Reduce.

@CrazyIvan359
Copy link
Owner Author

validate_item and possibly others are using a direct array comparison like itemRegistry.getItems("") === [] but this will never be true in JS.

Originally noted here

@volfan6415
Copy link

Have there been any updates to the Java helper libraires for OH3? While i am able to load these and get a basic rule to load it looks like a lot of the helper functions are still missing?
I see that adding the util.js functions back in is on the roadmap and happy to start working on that and create a pull request but didn't want to duplicate work.

@CrazyIvan359
Copy link
Owner Author

I have no major work done that is not available in this branch. At present I have no timeline to continue the work either.

That said, if you submit a PR to this branch I can merge it without much need to test myself since this branch is in development and there is no expectation of functionality yet.

@rkoshak
Copy link
Collaborator

rkoshak commented Jan 11, 2022

Would it make sense to just call this as done as it's going to be? Nashorn is not going to be supported by OH any longer once they move to Java 17 (or whatever they jump to) so maybe it makes sense to deprecate the JS libraries entirely and point people to JS Scripting.

We should obviously not remove the library for now so those with legacy systems and set ups that can't or wont move will still have access to it, but I feel like we should warn people away from starting with this now and I'm not sure continuing the work to bring it to parity with Jython is worth while any longer.

Jython's future too is murky but not as certain to go away as Nashorn so I'd encourage the continued maintenance and development on it. I don't think there ever was any Groovy stuff, but its future is probably the most certain of the three.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants