Move the whole LiveConnect initializion in "initStandardObjects" #1677
Conversation
| sb.append(name); | ||
| } | ||
| sb.append(JavaMembers.liveConnectSignature(argTypes)); | ||
| return sb.toString(); |
There was a problem hiding this comment.
this is a liveconnect only thing, so I moved it away from here
| | ScriptableObject.READONLY | ||
| | ScriptableObject.DONTENUM); | ||
| if (ScriptRuntime.isLiveConnectEnabled(scope)) { | ||
| if (javaException != null && isVisible(cx, javaException)) { |
There was a problem hiding this comment.
do not declare these additional properties, when the scope does not support liveConnect
| @@ -1749,7 +1749,8 @@ public static Object javaToJS(Object value, Scriptable scope, Context cx) { | |||
| if (value instanceof String | |||
| || value instanceof Number | |||
| || value instanceof Boolean | |||
| || value instanceof Scriptable) { | |||
| || value instanceof Scriptable | |||
| || value instanceof Undefined) { | |||
There was a problem hiding this comment.
do not pass undefined to wrapperFactory
There was a problem hiding this comment.
Can you elaborate on the 'why' of this?
There was a problem hiding this comment.
Undefined is already a js compatible object, so I think, it does not need a javaToJs conversion.
Background: I plan to make the wrapperFactory optional (no wrapperFactory == no liveConnect) and I found some failing tests, that have passed undefined here, although they do not use use liveConnect
If value is an instance of String, Number, Boolean, Function or Scriptable, it is returned
as it and will be treated as the corresponding JavaScript type of string, number, boolean,
function and object.
I will line up the code with the documentation. (Function mentioned in doc, but not handled in code) and maybe extract this in a separate commit.
|
I haven't read this yet, but I was wondering in general if it'd make sense to use the "ServiceLoader" Java facility to load optional classes during initialization. I was considering trying to do it for the XML implementation, and I could see how it'd also be helpful here, even if it's in the same module, and certainly in the future if it moves to a different module. It would let us drop in additional standard objects by just adding to the classpath, rather than messing around with reflection and stuff. https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/ServiceLoader.html |
|
@gbrail like that idea, as I foresee more optional add-ons in the future for Rhino, like a WASM implementation depending on a 3-rd party lib, EcmaScript intl-402 stuff that needs ICU4J etc. The ability to develop those outside the core Rhino engine and some auto-loading mechanism of they are on the classpath would make for a good facility I think |
|
Thanks for feedback. Yes that would be the long term plan: But first I would like to sort out some things. You may take a look in my experimental branch #1655 Maybe the xml stuff could also be a good candidate to move it to a module first |
This PR moves the initialization of all live-connect related objects in "initStandardObjects".
Ideally, we never need the wrapper factory when only dealing with plain javascript objects.