jasnell / proposal-istypes

TC-39 Proposal for additional is{Type} APIs
201 stars 7 forks source link

Rationale for userland objects being able to masquerade as a built-in? #29

Closed ethanresnick closed 7 years ago

ethanresnick commented 7 years ago

I understand that this proposal, by design, allow any object to masquerade as any of the built-ins. What is the rationale for that decision and should this rationale be added to the readme?

ljharb commented 7 years ago

Polyfills/shims and secure-realm code, for example, must be able to create builtins, remove them, or replace builtins that are noncompliant - as such, a shim (that runs before other code) must be able to create its own builtin replacement and truly masquerade as if it were the original builtin.

I think putting this in the readme is a great idea.

ethanresnick commented 7 years ago

Cool. Thanks Jordan!

Slayer95 commented 7 years ago

Doesn't this contradict the first requirement for the proposal?

Mechanism for reliably determining if any given object is a built-in or is an instance of a built-in, even across realms.

If we were able to rely on Builtin.typeof, it would be possible to implement shallow cloning of ES6 collections, regardless of real or polyfilled, as in the Gist (if Builtin.typeof(object) === 'Set', we iterate over [[SetData]] with Set.prototype.keys)

Enter writable @@builtin, and we now get a TypeError exception when trying to clone a polyfilled Set with the @@builtin value set. Gist

jasnell commented 7 years ago

Doesn't this contradict the first requirement for the proposal?

Yes, but that's an intentional and important compromise to allow for polyfills and shims as @ljharb explains. The mechanism is not intended to be foolproof, and if developer chooses to masquerade a given type, then the responsibility falls on the developer to do the right thing.

Slayer95 commented 7 years ago

It's precisely that "compromise" that breaks for shims the shallow clone use case I put forward.

If, after this proposal, developers still need to make sure that builtins are real builtins, and even ensure that @@builtin doesn't throw, then what's the overall benefit over just brand checking with Object.prototype.toString.call(), which behaves similarly with @@toStringTag?

Slayer95 commented 7 years ago

@ljharb , why isn't it an alternative for shims to just shadow broken methods?

ljharb commented 7 years ago

@Slayer95 shadowed method replacements don't have access to the internal slots that builtin ones do; often it can be necessary to replace the entire implementation when a single method is broken.

The issue is that any JS code can only be robust up to the limits enabled by previously-run JS code. So, the first JS ran might be polyfills/shims - which would supply Symbol.builtin values as needed. Then, application code might be run, which caches Symbol.builtin values so as to be robust against later modification. Finally, untrusted code might be run (like ads, or browser extensions, or userscripts), and those might modify Symbol.builtin in an attempt to have non-builtins masquerade as builtins - but in this scenario, the shim has ensured correctness, and the robust application code has ensured secure checks.

Slayer95 commented 7 years ago

The issue is that any JS code can only be robust up to the limits enabled by previously-run JS code.

Makes sense. From that line of thought one could conclude that there is no perfect solution to accomodate shims. Nevertheless, I'll take your word that the proposed one is the best.

As for the overall usefulness of this proposal?

what's the overall benefit over just brand checking with Object.prototype.toString.call(), which behaves similarly with @@toStringTag

This proposal was originally inspired on Array.isArray(), which is perfectly reliable, and I found it awesome back then. If I remember correctly, this proposal also used to include the (now withdrawn) Error.isError() proposal.

Utility functions that with 100% reliability check for the existence of builtin internal slots such as [[ErrorData]] -or [[SetData]] from my given example- are a real need IMO. However, I am afraid that its competition with the equally legitimate interest of first class shimming support will be a setback.

ljharb commented 7 years ago

fwiw, this proposal allows for Error.isError by providing a builtin function on Error.prototype[Symbol.builtin], and checking for [[SetData]] (while it can be done with a try/catch around a .call on the "size" accessor function), Set.prototype[Symbol.builtin] would provide it as well.