nextgenhealthcare / connect

The swiss army knife of healthcare integration.
Other
932 stars 280 forks source link

Rhino should be retired in favor of V8 #4409

Open MichaelLeeHobbs opened 4 years ago

MichaelLeeHobbs commented 4 years ago

Rhino should be retired in favor of V8

Why

Rhino has not aged well and is far behind modern JS. Rhino was considered "obsolete" in 2015 see: https://github.com/kangax/compat-table/pull/409. Things have not improved much since then: https://mozilla.github.io/rhino/compat/engines.html

At this point, Rhino is thoroughly obsolete and not like to ever be able to catch up to modern JS standards. All major browsers and server/workstation JS engines support 100% of JS features through 2019 and many of the 2020 features. There are a few exceptions to that statement but overall it is an accurate reflection of the state of JS. https://kangax.github.io/compat-table/es2016plus/

Advantages

Cons

Other factors to consider:

Path Foward

While this is not easy or painless I truly believe it is critical to the future of Connect and it's continuing dominance and growth in the HL7 space and beyond. With Connect using Nodejs and Express.js it would be trivially easy to build solid restful applications using Connects concepts and design methodology. While Connect was primarily created with HL7 in mind it is ripe to expand well beyond that. It only needs to modernize its scripting engine and it could easy complete with the likes of NodeRed and even beat NodeRed at its own game as Connect natively supports storing its channels in a database whereas NodeRed does not support storing flows in a database and has no addon that will give it that feature. As a long time user and support of Mirth/Connect please take this step to move to Connect forward.

tbshill commented 4 years ago

I'd like to express my support for this issue. I started using Mirth earlier this year because of how quickly I could build interfaces without having to wait for approval to purchase more connections. In addition, I prefer Mirth because I'm not limited to XSLT or drag-and-drop user interfaces. In my opinion, Mirth's scripting engine is what make it such a wonderful product. My only hesitation to making Mirth the primary integration engine at the company I work for is my low confidence in Rhino. As Rhino ages I feel a responsibility to migrate critical interfaces ( e.g HL7 communicating ER, NICU, or ICU data ) off of the product to something that is actively being maintained. Today Rhino is sufficient, but a couple years down the road it will be a liability.

If I may add to MichaelLeeHobbs's Path Forward: I think NodeJS could provide opportunities for developers to use Node's wonderful testing frameworks to create unit tests, and integration tests on their interfaces.

narupley commented 3 years ago

I can't promise this will be prioritized anytime soon, but I am looking closely at this. Currently Rhino is sort of "entrenched" in Connect in a couple ways, and the bulk of the work will not be simply replacing the library, but handling all the edge cases or places where we specifically tap into Rhino classes. For example all the ContextFactory and classloader stuff we do so that channels can have individual access to custom library resources.

Another big one is E4X. It's deprecated technically, and yet there doesn't seem to be any good replacement for it. How will users easily manipulate their XML-based transformed messages inside a filter/transformer? Maybe first we need to add JSON serialization to all data types as an option.

As far as I can tell Rhino isn't dead or obsolete though. It's still being actively developed, latest version came out just a few months ago. However yes, it hasn't kept up with the latest ECMAScript standards.

MichaelLeeHobbs commented 3 years ago

It is possible to replicate most e4x functionality with Proxy in es6. Some functionality is not possible with out transpiling the code with a tool like babel. Here is a sample of some code I have been working on that demonstrates the basic concept.

class Message {
  constructor(raw) {
    let hl7 = raw
    if (typeof raw === 'object') hl7 = raw.hl7
    const handler = {
      get(target, property, receiver) {
        // return Reflect.has(target, property) ? target[property] : `Not found: ${property}`
        if (Reflect.has(target, property)) return Reflect.get(...arguments)

        let key = hl7KeyParser(property)
        if (!key) return undefined

        // what do they want? segment, field, component, subcomponent
        // console.log(`Proxy.get - `, key)
        if (key.type === 'segment') return target.getSegment(key)
        if (key.type === 'field') return target.getField(key)
        if (key.type === 'component') return target.getComponent(key)
        if (key.type === 'subcomponent') return target.getSubcomponent(key)
      }
    }
    this._raw = hl7
    this._header = new Segment({message: this, name: 'MSH'})
    this.segments = []
    if (hl7) this.parse({msg: hl7})

    this._proxyThis = new Proxy(this, handler)
    return this._proxyThis
  }
  ...

This handles the specific case of msg['PID']['PID.2']['PID.2.1'] where any part of that may be undefined. Additionally, those sorts of handlers are not actually needed in es6 as the ?. operator can handle that as well. Including Proxy handling would make a transition much smoother for most existing code. However, Proxy does not address code like

for each(seg in msg..NTE){
    orderData.notesAndComments.push(seg['NTE.3']['NTE.3.1'].toString())
}

This would require transipiling. IMO should be deprecated.

At best Rhino is on life support.

Mozilla has unofficially deprecated it by archiving all or nearly all the pages. https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Rhino Due to how they archived the documentation pages most of them are broken now and it is very difficult to navigate the documentation.

Strikes against Rhino.

Key reasons to switch to V8. By switching you would instantly have access to 1,493,231 JS libraries along with the full power of Nodejs and the ability to import native compiled c/c++ and still have access to Java's estimated 373,794 libraries. V8 is the defacto JS engine even Microsoft recognized that when they switched Edge to Chromium.

I hate to even say this but you could add transipiling from es6 to es5 as a way to support most but not all modern JS code while still using Rhino. I can tell you from first hand experience debugging transipiled code is not fun. Combine that with Mirth's less than stellar stack traces and...

Rhino is a band aid that needs to be ripped off. The longer you wait the more painful it will be. At best Rhino is on life support. It would only take the loss of one or two of it's remaining devs to put it in the grave. Yes, this will be painful but I don't really think you have a choice if Connect is to remain viable and relevant for the next 10 years. It's only a matter of time before someone or some group takes the opensource parts already out there and builds a replacement in Nodejs or maybe Go.

kpalang commented 3 years ago

To keep the conversation going, here's a little something something https://github.com/nextgenhealthcare/connect/discussions/4650#discussioncomment-1072584