tc39 / proposal-array-is-template-object

TC39 proposal to identify tagged template string array objects
https://tc39.es/proposal-array-is-template-object/
MIT License
39 stars 7 forks source link

Changed to a method on a prototype. #16

Open koto opened 3 years ago

koto commented 3 years ago

Resolves #10.

ljharb commented 3 years ago

This change seems like it warrants discussion in plenary.

koto commented 3 years ago

Agreed, I'm not merging yet. I intend to present the issue today, and if the plenary decides to go with this variant, ask for a stage 3 re-review (either by Thursday, or postponed to March. Hoping for Thursday though).

littledan commented 3 years ago

This was discussed in plenary. @erights agreed that it resolved one of his multiple issues, and no one raised concerns. I think it would be helpful to land this patch, given that feedback.

ExE-Boss commented 3 years ago

This will not actually change anything.

And in fact, will only make things more annoying, as a truthy isTemplateObject property will almost certainly have to be added to Array.prototype[Symbol.unscopables].


Also, the following is still broken when a Proxy is passed: https://github.com/tc39/proposal-array-is-template-object/blob/1d52f7dadf7eeb78c5e8da7c9825165e1c1f863c/spec.emu#L67-L72

This is because a Proxy object doesn’t have a [[TemplateObject]] internal slot.

koto commented 3 years ago

This will not actually change anything.

Could you elaborate? This was introduced to address https://github.com/tc39/proposal-array-is-template-object/issues/10#issuecomment-767063653 (practical membrane transparency argument) in specific. What does it not change that needs changing?

And in fact, will only make things more annoying, as a truthy isTemplateObject property will almost certainly have to be added to Array.prototype[Symbol.unscopables].

Good catch!

Also, the following is still broken when a Proxy is passed:

https://github.com/tc39/proposal-array-is-template-object/blob/1d52f7dadf7eeb78c5e8da7c9825165e1c1f863c/spec.emu#L67-L72

This is because a Proxy object doesn’t have a [[TemplateObject]] internal slot.

That is intentional. isTemplateObject does not punch through proxies, the arguments for that behavior are in #10.

ExE-Boss commented 3 years ago

I mean, the steps should be written as:

  1. If IsArray(O) is true and O has a [[TemplateObject]] internal slot and O.[[TemplateObject]] is true, return true.
  2. Return false.

Also,

function doStuff(template, ...args) {
    if (!template.isTemplateObject()) {
        throw new TypeError("Not a template object");
    }

    // do stuff with `template` and `args`
}

depends on template having a isTemplateObject method, whereas:

function doStuff(template, ...args) {
    if (!Array.isTemplateObject(template)) {
        throw new TypeError("Not a template object");
    }

    // do stuff with `template` and `args`
}

doesn’t depend on template’s prototype chain and is still realm‑agnostic.

ljharb commented 3 years ago

Every array would have that slot, so IsArray would be sufficient.

koto commented 3 years ago

(realm-agnostic) Array.isTemplateObject was the initial proposal, to which @erights raised a concern that it violates "practical membrane transparency". Moving to prototype is a way to obviate this concern; the downside is indeed that most likely one needs an existence check, or capturing the prototype function earlier, not to have runtime errors. The tradeoffs are discussed at length in #10.