Object.assign is broken after 1.7.15 with custom ScriptableObjects

pavelhoral commented 2 months ago

The issue

This is related to #780, which changed how Object.assign works. That change makes sense as it aligns the implementation with the ECMA specification. However it is a breaking change for all custom ScriptableObject based classes (not only inside RhinoJS itself, but in depending projects as well).

Symptoms

Classes extending ScriptableObject quite often only override basic get / put / has / getIds methods. Trying to use Object.assign ends with error due to missing attribute slot:

js> var customNative = new Packages.org.example.javascript.CustomNative()
js> Object.assign({}, customNative)
js: Property hello not found.
        at (unknown):18

Quick solution to the issue

The first solution that comes to my mind is to make sure that missing attribute slot does not end with exception... at least not in case when called from Object.assign as the specification clearly states the following:

So undefined property attributes is a well defined state... the property should be skipped.

Proper solution

Previous quick solution still means 1.7.15+ brought breaking changes for custom ScriptableObject implementations. I would appreciate if there would be clear example how custom NativeObject / IdScriptableObject / ScriptableObject should be implemented... in the case of IdScriptableObject implementation I guess one needs to override findInstanceIdInfo method?

Footnote

Not sure if this is a partial duplicate of #1549. That issue's description is a bit cryptic for me.

p-bakker commented 1 month ago

Added the docs tag as well, as in the end, we'll need to update the docs for implementing custom host objects as well: currently all examples are about implementing the Scriptable interface, but while that works, it doesn't result in host objects that extend Object, which is a best practice nowadays

pavelhoral commented 1 month ago

I was playing with this a bit in a project that is using RhinoJS and I am not able to fix it on my end as NativeObject's assign method calls package private method putImpl that I am not able to override:

https://github.com/mozilla/rhino/blob/cbb0e2f17019e75eae9f3bc04745dcba4f5a0bac/rhino/src/main/java/org/mozilla/javascript/NativeObject.java#L674

https://github.com/mozilla/rhino/blob/cbb0e2f17019e75eae9f3bc04745dcba4f5a0bac/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java#L210

https://github.com/mozilla/rhino/blob/cbb0e2f17019e75eae9f3bc04745dcba4f5a0bac/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java#L2510

The ScriptableObject#putImpl method is tightly coupled with the private slotMap property. This prevents subclasses to intercept such call (e.g. when the object represents custom java.util.Map wrappers).

mozilla / rhino