[draft] first draft for isolated realms #291
Conversation
There was a problem hiding this comment.
This change looks like a "poor man's membrane" to me. I strongly suggest we should keep the goal of the membrane in mind though we can ship this Function or primitive only membrane for now. (which means we should not add anything that might conflict with the membrane if we need to add it in the future).
And I don't think removing the original unwrapped version is a good idea. This limits the full power of Realms API. Please also keep the old version, and make the Isolated version as an opt-in advance feature.
spec.html
Outdated
|
|
||
| <dd> | ||
| <ul> | ||
| <li>The completion value must be the exported value for the _bindingName_ associated to the Module Namespace Object.</li> |
There was a problem hiding this comment.
Does it have the live binding?
There was a problem hiding this comment.
that's correct, it is not a declaration of a binding, it is just invoking an API to resolve to a value.
spec.html
Outdated
| 1. Set _argumentsList_[_key_] to ? GetWrappedValue(_targetRealm_, _o_). | ||
| 1. Let _wrappedThisArgument_ to ? GetWrappedValue(_targetRealm_, _thisArgument_). | ||
| 1. Let _value_ be ? Call(_target_, _wrappedThisArgument_, _argumentsList_). | ||
| 1. Return ? GetWrappedValue(_callerRealm_, _value_). |
There was a problem hiding this comment.
A poor man's membrane. I need to code in this style to access objects in the other realm.
const argParam = { a: 1 }
const returnValueGetter = wrappedFunction(name => argParam[name])
const x = returnValueGetter("x")
There was a problem hiding this comment.
I have no idea what you mean by this code, but I'm pretty sure you can use the current API to get wrapped functions and primitive values. We hope that complex structures such as argParam can eventually be transferred as Records and Tuples, but that depends on the respective proposal to land.
Also, this is a discussion already existing in the other issues. Bringing this discussion here unnecessarily expands the complexity of integrating this PR into the main branch. Let's use the proper channels.
| 1. Assert: _callerRealm_ is a Realm Record. | ||
| 1. If Type(_value_) is Object, then | ||
| 1. If IsCallable(_value_) is *false*, throw a TypeError exception. | ||
| 1. Return ? WrappedFunctionCreate(_callerRealm_, _value_). |
There was a problem hiding this comment.
It will return a different function even the _value_-_callerRealm_ pair is the same.
There was a problem hiding this comment.
that is by design as explained in #289, this is not a membrane, it does not do identity reconciliation. Every time it sees a callable coming thru the boundary, a new wrapped function is created.
There was a problem hiding this comment.
If we don't keep identity reconciliation, we will never be able to make it after we ship it cause it's breaking change. That will block the possibility to the membrane
There was a problem hiding this comment.
No, that's not true, you can implement identity preserving membrane on top of this.
There was a problem hiding this comment.
It would be the responsibility of the membrane or library sitting on top of this API to avoid recreating wrappers.
Given that symbols are unique and comparable through round trips, wrapper functions provide indirect cross realms references, and WeakRefs are available, it should be possible to build a full non-leaky proxy based membrane. I believe the next step is to validate this by building such a membrane on top of this proposed API. See leobalter/shadowrealms-polyfill#13
There was a problem hiding this comment.
One question however is if the GetWrappedValue operation should automatically unwrap a wrapped function if its [[Realm]] matches callerRealm. Right now it would double wrap it.
There was a problem hiding this comment.
I'm opening an issue to discuss if we should unwrap or double wrap it.
I don't see much value or the goal in sending the same function back and forth. If the goal is to get a unique identity, symbols should resolve this trick in a much more lightweight manner.
Adding an extra check for every function to see if it can be unwrapped might end with some cost, but at the same time, it cuts a chain of references here and there.
|
Thanks for the review @Jack-Works.
As much as I want to have full power of Realms API, the goal is to land something we can work with. The isolated realms is good enough and provides tools that enables the goals of this proposal, without being challenged by the invariants once presented to the champions and TC39 that historically prevented advancement to Stage 3. I'll cheer up if someone wants to take the previous Realms API forward, but as a champion of this proposal, I'd rather stick with the isolated Realms for now and explore membranes over the given API. To be honest, I like the path we are going towards avoiding identity discontinuity. We are setting a good exploration path towards injecting code in the realms and that's the field I'd like for us to explore more next. Please refrain the goal of this PR is just to update the current spec draft of the proposal. I'd like to ask to avoid using this thread to discuss the general directions of this proposal. At the same time, I'm requesting we stick with all the TC39 conversation we already had about this and what lead us to where we are right now. |
leobalter
changed the title
|
@leobalter @Jack-Works I have fixed the issues you have pointed out, this is ready for another pass. |
Spec'ing the result of the discussion from #289 to describe the isolated realms.