Ergonomics on WrappedFunction thisArgument throwing when it is an object

[legendecas]

As for now, WrappedFunction.[[Call]] converts the thisArgument to wrapped value and then passes the wrapped this argument to the target function in the other realm. When this behavior comes to life, the following common pattern can be very awkward:

const shadowRealm = new ShadowRealm()
shadowRealm.evaluate('...') // <= setup the shadow realm.
const store = {
  foo: shadowRealm.evaluate('globalThis.foo'),
  bar: shadowRealm.evaluate('globalThis.bar'),
}

store.foo() // <= TypeError: `this` is an non-callable object and can not be wrapped!

So people have to make sure they call the wrapped function without this binding, like:

const {foo} = store
foo()

// or
store.foo.call(undefined)

This makes calling the wrapped function less ergonomic and cumbersome to be correct. Since the wrapped function can not be called with new as it doesn't have [[Construct]] and it doesn't make sense to return an object from new operator for wrapped function, maybe we should omit the thisArgument and bind this with dedicated values like undefined?

[caridy]

I'm supportive of this reasoning. I'm curious what @erights thinks of it?

[erights]

Yes, I also support it, but only with the current conclusion: that the this binding passed in be undefined. I emphasize this because that conclusion above is stated only with a "maybe". (And I also saw an earlier version with the abhorrent choice of a global object. Absolutely not. Glad to see that's already gone!)

Note that this concerns only the this binding passed in. How that affects the this binding seen by the code of the invoked function depends on the function. If an arrow function, then it sees its own lexically bound this. If a sloppy function, then it does see its own global object. Functions declared with function, concise methods, and builtins do indeed see undefined. Functions declared with class have no [[Call]] behavior so it is not an issue. (Assuming we cannot new across the callable boundary, which I hope is true! @caridy? )

[ljharb]

What happens if i do store.foo.call(1), and the foo function was defined in strict mode? I would expect to receive the primitive as the receiver.

[mhofman]

Would people really expect that? These functions are already special and don't let non-primitive arguments or return values through. We can very well say they don't support passing any this. If the receiver expect a this, one can create an uncurried version just for the callable boundary.

[ljharb]

It's the way JS works already, and TC39 took the time to make that change when adding strict mode, so it seems like it should require a large burden to change it.

This proposal's callable are already bizarre in a number of ways; it would be great to reduce the otherwise-valid use cases they don't work with.

[leobalter]

@ljharb We can set [[ThisMode]] of the wrapped function exotic to lexical, WDYT?

[leobalter]

sounds like just changing the following in the wrapped function [[Call]] works:

-7. Let wrappedThisArgument to ? GetWrappedValue(targetRealm, thisArgument).
-8. Let result be the Completion Record of Call(target, wrappedThisArgument, wrappedArgs).
+7. Let result be the Completion Record of Call(target, undefined, wrappedArgs).

expanding the case above things seem ok:

const shadowRealm = new ShadowRealm();
const whatIsThisWrapped = shadowRealm.evaluate(`
  (function whatIsThis() {
    return this === globalThis;
  });
`);
const whatIsThisStrictWrapped = shadowRealm.evaluate(`
  (function whatIsThis() {
    "use strict";
    return this === undefined;
  });
`);

const store = {
  whatIsThisWrapped,
  whatIsThisStrictWrapped
};

store.whatIsThisWrapped(); // true
store.whatIsThisStrictWrapped(); // true

As pointed out by @legendecas, the last two lines above are a TypeError today just because the wrapped function being set as the value of an object property.

The spec change also allows:

whatIsThisWrapped.call([]); // true
whatIsThisStrictWrapped.call([]); // true

While the only downside is not being able to transfer a this value to the other realm:

whatIsThisWrapped.call(1); // true
whatIsThisStrictWrapped.call(1); // true

[mhofman]

It's the way JS works already, and TC39 took the time to make that change when adding strict mode, so it seems like it should require a large burden to change it.

But it's not a change. It's a different behavior for a new feature / addition.

One doesn't expect all functions, to receive an explicit thisArgument (e.g. arrow or bound functions).

Code has to explicitly receive a function from another ShadowRealm, initially through .evaluate() or .importValue(). Those functions simply have different behavior than plain function objects.

@ljharb We can set [[ThisMode]] of the wrapped function exotic to lexical, WDYT?

What would the lexical this be? Wouldn't that have the same effect of preventing an explicit thisArgument from being passed through, which I believe is @ljharb's concern?

[mhofman]

To clarify, I'm not particularly in favor of one or the other. IMO this is a low level API, and the program has to have different expectations when working with these wrapped functions:

• Currently they cannot take the shorthand of store.wrappedFooFromOtherRealm() (TypeError thrown), but can explicitly do wrappedFooFromOtherRealm.call(1).
• With the proposed change, programs can do store.wrappedFooFromOtherRealm() and have the wrapped function behave more-or-less like a function bound to undefined, but they can no longer do wrappedFooFromOtherRealm.call(1) (thisArgument silently dropped, like bound functions). They have to do wrappedUncurriedFooFromOtherRealm(1).

[leobalter]

@mhofman I was wrong and the other spec change is way more effective than setting [[ThisMode]] as this is not necessary to resolve the problem here.

In any case, not observing the this value from .bind, .call, etc. already has precedent from Arrow Functions.

[leobalter]

Agreed. There are pros and cons for both sides, but for what I want in the wrapped functions everything seems low level and I'm aware that wrapped just chains the argument values to another function call inside the shadowrealm and get the result. I don't feel like we should encourage one to also transfer values using this.

[ljharb]

Wrapping a non-arrow function and getting arrow function semantics seems very strange to me - especially if the function uses its own prototype inside realm, which would be totally valid. Imagine, inside the realm:

function f() {
  return !!f.prototype
};

Inside the realm, calling f() would return true. From outside the realm, suddenly it would return false?

If not (and i'd hope not), then it's not an arrow function, and it can't be explained as "it's like an arrow function, that's why this doesn't work as expected".

[mhofman]

It can be explained as a function automatically bound to undefined.

[leobalter]

f() would always return true as this is only discarded in the wrapped function, the f, I assume from inside the ShadowRealm.

Borrowing semantics similar to arrow function work just as finding precedent, but it doesn't mean the wrapped functions will be presented like these. @mhofman said they can be considered as bound to undefined.

[ljharb]

That explanation holds, at least, but that still doesn't explain why we'd want to prevent something that could otherwise work.

[leobalter]

@ljharb There are pros and cons, and I prefer what this change would provide.

With the proposed change we can place the wrapped function as values of objects and avoid surprises such as store.foo() throwing a TypeError. I'd find this error very confusing.

The status quo allow setting custom this values as long as they are primitives or callables, but still can't set this to any non-callable object, which seems pretty annoying...

// status quo
store.foo(); // TypeError
store.foo.call(undefined); // works
store.foo.call(1); // works
store.foo.call([]); // TypeError

class Store {
  static foo = store.foo
}

Store.foo(); // works, not a TypeError with the current spec text

store.foo.call(store.foo); // works
store.foo.call(function() {}); // works

store.foo.bind(undefined)(); // works
const boundFoo = store.foo.bind(store); // works!!
boundFoo(); // TypeError

With the proposed change, all the TypeErrors listed above are removed, but the this value will be ignored when calling the target function.

[Jack-Works]

Is it possible to make it a runtime error when the wrapped function tries to access this?

const a = {
    foo: realm.evaluate('function () { this }'),
    foo2: realm.evaluate('function () { return 1 }')
}

a.foo2() // 1
a.foo() // TypeError

[legendecas]

@Jack-Works I didn't find throwing on accessing this can help to alleviate the situation. functions should already be aware that their this argument can be arbitrary values and they can be bound to get expected static this binding. However, throwing on accessing this makes the this binding behavior of function, arrow function, and bound function drastically diverge, which can be more problematic than simply binding this to undefined.

[caridy]

In the spec, I don't see anything that can help us here except for a revoked proxy, but the incoherence with arguments, which are going to be wrapped before calling the function and throwing if needed, and the this to only throw when interacting with is a bigger issue for me.

[leobalter]

@Jack-Works it seems like your suggestion would need a separate and probably more challenging discussion. I agree with @legendecas and @caridy and I don't want it for this API. This would require shadowrealms evaluating the function body of anything returned/wrapped.

---

I believe the challenge of this issue is changing the wrapped function to ignore this value and only chaining the arguments to the target function in the other realm. I tried showing above an expansion of pros and cons if we do this change.

While the champions showed positive interest in this change, it seems we need consensus from @ljharb who needs to be onboard with it. I'd like to pull the discussion back to this topic.

[leobalter]

I confirmed with @ljharb we don't have consensus for this change yet. I don't think this would or should block the proposal, but I hope we can eventually work through accepting the change.

[chris-kruining]

I have a question, which I assume I am wrong about, but wanted to ask it anyway:

Currently I am having trouble with a proxy and private class fields. wouldn't the wrapping and/or removing of the this in functions calls not suffer from the same issue of privates becoming inaccessible?

[ljharb]

@chris-kruining Nothing with a private field will be able to cross a shadow realm and end up being an object with the same private field - the issue you’re referring to is an unavoidable one with or without this change.

[bathos]

I’ve experimented with adapting some existing iframe-realm-based code to ShadowRealm in D8 in the last few weeks and have found this behavior to be nothing but (very frequent) surprise pain in practice. I can imagine scenarios where it could be desirable, but if this behavior is to remain, please consider making WrappedFunctionCreate / WFEO.[[Call]] take into account the target function’s [[ThisMode]]/BFEO-ness. Most of the time when I’ve hit this, the target function was an arrow and in such cases, the throwing behavior can never be helpful and is super baffling.

// e.g. it absolutely hates when you write a membrane bootstrapper thing as a class

#shadow = new ShadowRealm;
#evaluate = this.#shadow.evaluate(`() => {
  /* non-global scope setup stuff here */
  return s => eval(s);
}`)();

this.#evaluate("'💀'"); // throws
(0, this.#evaluate)("'🥲'"); // i guess eval just loves to find a way to be inside this expression

[mhofman]

That's an interesting idea. To make sure I got that right, if the target of the WFEO is an arrow function, then it wouldn't try to wrap the this value and just ignore it? I'd have to think if this would expose any new capability to sense what kind of function you're holding.

[caridy]

please consider making WrappedFunctionCreate / WFEO.[[Call]] take into account the target function’s [[ThisMode]]/BFEO-ness.

@bathos I think I'm fine with that suggestion since the arrow function can't really observe the this value anyways. There are a couple of notes though:

1. A proxy of an arrow function can still sense the thisArg value, which can be seen like a prior art here.

const af = new Proxy(() => {}, { apply(target, thisArg, args) { console.log(thisArg) } })
> undefined
Reflect.apply(af, {x:1}, [])
> VM519:1 {x: 1}

2. in terms of observability, @mhofman is right, this is a new way to observe what's being exposed/shared from the shadow realm. Today, all WFEO are equal and somehow opaque.

[bathos]

this is a new way to observe what's being exposed/shared from the shadow realm

That’s true — though it seems like it’d stop short of constituting a real “generalizable” brand check right? Like, the kind suitable for use with arbitrary values about which you hold no prior knowledge. The receiver conversion step is currently the last point in WFEO.[[Calll]] that can be reliably caller-observed before WFEO.[[WrappedTargetFunction]].[[Call]] is entered. If that same step became conditional (but did not move relative to the others), the last point that can be caller-observed also becomes conditional and so is no longer reliable. So a novel observation is possible, but not an observation that you can be sure in advance will not have unknown external effects. If you ~inject your GOTO statement~ throw an abrupt completion in the conditional step, you can only exit if the answer is “no”, but if you throw prior to it you never learn the answer to begin with. So the theoretically newly-observable “Arrow or BFEO” brand ends up inextricable from the risk of entering other-people’s-side-effects town. That makes it pretty useless for everything interesting AFAICT, but perhaps I’m not being imaginative enough.

(GetFunctionRealm happens before any of the conversions, so you can already write a reliable “does this WFEO’s [[WrappedTargetFunction]] function terminate with a revoked PEO” test using WFEO.[[Call]]. Previous language features already left this status reliably observable by other means, so it’s not a problem AFAIK, or avoidable, but it’s the same order-of-observable-ops-reveals-information-about-a-slotted-value thing via function arguments that observe their own conversion via "name" and "length" properties).

---

In any case I’m hesitant about my own suggestion due to likely-still-common code transformations from () => {} to function() {} and vice versa. Often those conversions are already unsound, but in practice/usage context are usually not problematic. Here those code conversions would be a more substantial problem.

[mhofman]

While implementing ShadowRealms I've also been playing with it a bit.

I'm curious about the design decision around the WrappedFunction [[Call]] internal method. That method wraps the this value; which then requires that this be either a primitive or callable.

This can lead to some surprising behaviour; for example, you cannot do setTimeout on a wrapped function object, because it explicitly sets the this value to an object, which isn't callable

var h = realm.evaluate('() => log("Heyho")') 
setTimeout(h, 0); // Throws on execution.

In general, given that from the perspective of the target function, the this value will always either a primitive or a wrapped function... there's relatively little to the target function by the this value; which makes me wonder: should the [[Call]] internal method instead provide some sensible default this (i.e. undefined), or is there a way in which wrapping this is actually important?

I mean, I suppose you could imagine j = realm.evaluate('function f() { this("why"); }; f'); j.call(console.log), but I'm not sure about prioritizing that usage over the ability to pass a WrappedFunction object to APIs that may try to invoke it with a different (object) this.

Originally posted by @mgaudet in https://github.com/tc39/proposal-shadowrealm/issues/361

[mhofman]

One issue with detecting the target's mode at call time is that it'd require recursion for nested WFEO. I suppose we could detect the target's mode at creation time and store a "target ignores this value" flag in a slot of the WFEO. Then when detecting the target at creation, it'd set the flag for BFEOs or lexical thisMode ordinary (aka arrow) functions targets, or copy the flag of a WFEO target.

[bathos]

That’s a particularly interesting example. Web IDL’s “regular operations” implement “undefined → global” behavior similar to sloppy functions so that if they are members of a [Global] interface, their receiver-sensitivity is “masked.” As a consequence folks don’t usually have a reason to learn that setTimeout.call(1, () => {}) or atob.call({}, "") fail brand checks, unlike ES global “methods” like parseFloat. It’s far easier to learn that here by accident, but always setting the receiver to undefined would allow them to not include this surprise behavior as the original realm’s global fallback would still be used.

Note that my prior suggestion would not help with setTimeout and co.

[mhofman]

Note that my prior suggestion would not help with setTimeout and co.

I fail to see why not?

[bathos]

@mhofman sorry, that was unclear: it would help with the example given because that example is about how setTimeout passes a receiver that’s an object when invoking the callback. What it would not help with is setTimeout itself, which is receiver-sensitive, too, but treats undefined as though it were its realm’s global.

shadow.evaluate("provisioned => { globalThis.setTimeout = provisioned }")(setTimeout);

// in the shadow later, this throws in sloppy mode even if arrows were exempted from receiver-mapping:

setTimeout(() => {}, 0);

though I suppose that issue is not actually unique to global interface ops, thinking about it more

[mhofman]

Right, I believe all host functions are exotic, so I would not expect them to fall under this intervention. Same with proxies of functions. At the end of the day this is to make the user's bound and arrow functions more ergonomic through the callable boundary, nothing more.

Of course I'm still open to simply saying the receiver doesn't get wrapped and becomes undefined, but that does raise other issues. For example, a wrapped sloppy function (aka global thisMode) called with a primitive or callable as receiver would end up with its realm's global object as this on the other side, which could be surprising. This is admittedly a very edge case.