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

Examples are not emulating frames #268

Closed
annevk opened this issue Aug 6, 2020 · 15 comments
Closed

Examples are not emulating frames #268

annevk opened this issue Aug 6, 2020 · 15 comments

Comments

@annevk
Copy link
Member

annevk commented Aug 6, 2020

It seems to me that if you do something like realmGlobal.document = new FakeDocument(); you are not emulating frames as FakeDocument will have the "wrong" global associated with it.

@erights
Copy link
Collaborator

erights commented Aug 6, 2020

Hi @annevk ,

Given that FakeDocument is attempting to uphold the illusion, how can you tell? What's the observable difference that the FakeDocument cannot hide?

@annevk
Copy link
Member Author

annevk commented Aug 6, 2020

console.log(document instanceof Object) would be false.

@littledan
Copy link
Member

This does seem to be an error in the code samples, which don't pass the Realm into some constructors, so they couldn't possibly get this right.

The easiest approach may be to use Realm.prototype.import to run the emulation code within the Realm, so you automatically close over the right globals. Another approach is to be very careful and fix everything up in JS, based on passing the Realm in to the emulation code constructor.

Either way, sounds like the code samples should pass the Realm around to functions which would emulate the DOM to allow the prototypes to be correct.

@annevk Do you see this as a cause for more general concern, or would fixing up the code samples be sufficient?

@annevk
Copy link
Member Author

annevk commented Aug 6, 2020

Not sure, but it seems very easy to do the "wrong" thing here.

@littledan
Copy link
Member

littledan commented Aug 6, 2020

I think it's easy to do the "wrong" thing just about every step of the way in implementing/emulating Web APIs. For example, it's very easy to implement WebIDL coercions wrong in a way that has significant interoperability impact. The important thing is that we have solutions to these problems: for WebIDL, the pattern of automatic bindings generation, and for closing over the right Realm, we have the ability to evaluate code in the context of that Realm.

@leobalter
Copy link
Member

leobalter commented Aug 6, 2020

Hi @annevk!

It seems to me that if you do something like realmGlobal.document = new FakeDocument(); you are not emulating frames as FakeDocument will have the "wrong" global associated with it.

This is a known characteristic of Realms in ECMAScript, IMO, regardless of the Realms API.

That's also one the reasons this API encourages the usage of Realm#import() and has no immediate Node vm's .runInContext or evaluation method.

If the code injection from the incubator is still necessary, things like this FakeDocument example could still be tackled to preserve the new Realm identity, like:

const realm = new Realm();

await realm.import('fake-document.js');
const { FakeDocument } = realm.globalThis;

realm.globalThis.document  = new FakeDocument();

I'm not sure if this would be the recommended approach, I'd probably reuse the realm import myself.

I believe this also matches everything @littledan has said in his comment above.

We might update the examples to reflect possible effects of injecting code from different realms, which are not much different from iframes.

@caridy
Copy link
Collaborator

caridy commented Aug 6, 2020

Not sure, but it seems very easy to do the "wrong" thing here.

@annevk yes, you're absolutely right, it is easy to get in trouble when using such a low level API as Realms. Anything that involves multiple realms is always tricky, we call this the identity discontinuity hazard. I believe we have highlighted that in the proposal from the very early stages few years ago.

There are strategies to deal with this kind of problems in realms. One of them is what @littledan and @leobalter explained above. Another strategy is to use a proxy membrane, which we currently use in production with iframes to virtualize the DOM while preserving the identity of the objects.

@leobalter
Copy link
Member

@annevk I understand the original example represented an anti-pattern and therefore does not reflect our goals.

We modified the explainer to remove that example and added a new one where we implicitly expect a framework to handle cross-realm identity problems. The new example reflects better our goal in that section where we want to show a quick abstraction on how a framework can mock the DOM.

Ref commit: a2d5b82
Rendered: https://github.com/tc39/proposal-realms/blob/main/explainer.md#dom-mocking

WDYT? Please let us know if this is a better fit.

@annevk
Copy link
Member Author

annevk commented Aug 14, 2020

It seems that mostly sidesteps the issue. It doesn't show how you'd avoid it. Or is installFakeDOM() defined/explained somewhere?

@leobalter
Copy link
Member

@annevk it is not defined. The goal was to remove the example showing a bad practice, as it is implicitly expected for a proper framework/library implementation to understand and avoid cross-realms issues.

There are different ways to avoid the original problem of direct injection of objects into a different realm. We tried not to pick a favorite.

@caridy
Copy link
Collaborator

caridy commented Aug 15, 2020

installFakeDOM() defined/explained somewhere?

@annevk here is some pseudo-code for it (one of the many variations)

function installFakeDOM(realmGlobalThis) {
    const someRealmIntrinsicsNeededForWrappers = extractIntrinsicsFromGlobal(realmGlobalThis);
    Object.defineProperties({
         document: createFakeDocumentDescriptor(someRealmIntrinsicsNeededForWrappers),
         Element: createFakeElementDescriptor(someRealmIntrinsicsNeededForWrappers),
         Node: ...
         ... // all necessary DOM related globals should be defined here
    });
}

function createFakeDocumentDescriptor(someIntrinsics) {
     return {
          enumerable: true,
          configurable: false,
          get: new someIntrinsics.Proxy(document, createHandlerWithDistortionsForDocument(someIntrinsics));
     };
}

function extractIntrinsicsFromGlobal(realmGlobalThis) {
   return {
       Proxy: realmGlobalThis.Proxy,
       ObjectPrototype: Object.prototype,
       create: Object.create,
       ... // whatever you need to facilitate the creation of proper identities for the fake DOM
   };
}

That's one option to use proxies, notice that the Proxy constructor used is from the Realm, same for any proxy handler, or object/function/array accessible from inside the realm (e.g.: the one produced by createHandlerWithDistortionsForDocument should be an object with __proto__ set to realmGlobalThis.Object.prototype, this helps with the errors identity). Again, identity issues are hard to solve when multiple realms are playing together, but libraries and frameworks can tackle that at the lower level.

@leobalter
Copy link
Member

@annevk I'd like to check with you here on the current status. Thanks in advance!

@annevk
Copy link
Member Author

annevk commented Oct 8, 2020

It seems reasonable, but it's also an enormous increase in complexity meaning this API would be incredibly hard to use correctly for its stated purpose.

@leobalter
Copy link
Member

Thanks, @annevk!

I agree there is a complexity, but I'm not sure what the increase relates to. There are some aspects here about it we already handle today.

The example Caridy shows is similar to a solution we already have today, without Realms, it doesn't change much. The biggest win here is being able to do virtualization that is not finger-printable by the unforgeables (window.location, window.top, ...).

Another aspect of this proposal, as it's an ECMAScript proposal, it does not set governance over what the host provides. We could work out a html/dom integration that provides the API, even if we prefer a cleaner state. Our concerns are mostly within the unforgeables. If you checkout the Realm constructor API, it calls the [SetDefaultGlobalBindings](https://tc39.es/ecma262/#sec-setdefaultglobalbindings] abstract that captures the properties from the Global object that may have host defined properties in addition to the properties defined in this specification.

This proposal can't assume yet a final decision for browsers integration, same goes for other non-browsers implementations where it's ok to tailor the globals as adequate for each host.


Back to the complexity, yes, there is complexity here that I expect to be mitigated through usage of frameworks, some that are already ready when we do have Realms available. For anything else, like a quick userland code playgrounds, I expect the initial set of intrinsics would be great for experimentations and new opportunities. I see a lot of this usage for Testing APIs.

IMO, the complexity is within one trying to emulate the whole DOM APIs, that's a lot of good work done there for years. Even though, I expect most developers to only need fragments of it, when it's needed.

I value your feedback and maybe there might be other examples we can provide to mitigate your concerns.

leobalter added a commit that referenced this issue Oct 15, 2020
@leobalter
Copy link
Member

Since the last comment we've had some modifications in the explainer and the api redesign. I believe there isn't any further action for this one at this point, but we might eventually write something about the membranes integration and strategies for virtualization using the new API.

Thanks!

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

No branches or pull requests

5 participants