tc39 / proposal-rm-builtin-subclassing

Remove ES6 built-in subclassing
33 stars 5 forks source link

What is the difference between Type 2 and Type 3? #6

Open jridgewell opened 4 years ago

jridgewell commented 4 years ago

They both read the same to me. Does Type 2 only allow the exact subclass, while Type 3 allows any subclass?

rkirsling commented 4 years ago

Type 2:

class A extends Array {}
A.from([1,2,3])     // return type: A
  .map(x => x + 1); // return type: A

Type 3:

class B extends Array { static [Symbol.species] = Array; }
B.from([1,2,3])     // return type: B
  .map(x => x + 1); // return type: Array
syg commented 4 years ago

To add to what @rkirsling said:

littledan commented 4 years ago

It's hard for me to understand this distinction from the text in the explainer, since both II and III mention @@species. Is this a typo, in the case if II? Is the intention that II would use the dynamic value of this.constructor?

Ultimately, I think the difference between this.constructor and this.constructor[Symbol.species] (if that's what we're talking about for II vs III) is pretty superficial: both involve invoking arbitrary user-defined code (imagine if this.constructor is a getter), and use the result of that to do the following operations.

My understanding of the security issues in the past is, some come from invoking arbitrary user-defined code, and some come from manipulations on the object afterwards. I'm not sure how much we'd gain by just removing @@species and not removing this.constructor.

(By contrast, I think #1, which isn't one of the options mentioned in the current README, would address the second class of bugs around manipulating objects which are not actual instances of the superclass.)

If we get rid of @@species but keep the this.constructor logic, we have to ensure we have some other mechanism to make sure that Array methods, when called on things that are not Array subclasses (e.g., NodeList, or Objects with indexed properties).

syg commented 4 years ago

It's hard for me to understand this distinction from the text in the explainer, since both II and III mention @@species. Is this a typo, in the case if II?

Not a typo, but I see the confusion. The explainer's language distinguishes this by saying "default values of @@species" vs "custom values of @@species".

Ultimately, I think the difference between this.constructor and this.constructor[Symbol.species] (if that's what we're talking about for II vs III) is pretty superficial: both involve invoking arbitrary user-defined code (imagine if this.constructor is a getter), and use the result of that to do the following operations.

Yeah, I agree that from an implementation and a security POV the difference is superficial. The difference between Type II and Type III is the user-facing question: do users expect what class the instances that built-ins create to be programmable?

But that difference isn't exactly the constructor vs @@species difference. Indeed, you point out other ways to kind of satisfy the subclassing expectations in #1 without offering the full expressivity delegating completely to constructor or @@species.

My understanding of the security issues in the past is, some come from invoking arbitrary user-defined code, and some come from manipulations on the object afterwards. I'm not sure how much we'd gain by just removing @@species and not removing this.constructor.

(By contrast, I think #1, which isn't one of the options mentioned in the current README, would address the second class of bugs around manipulating objects which are not actual instances of the superclass.)

True, I'll try to incorporate #1 into the explainer somehow as an alternative.

If we get rid of @@species but keep the this.constructor logic, we have to ensure we have some other mechanism to make sure that Array methods, when called on things that are not Array subclasses (e.g., NodeList, or Objects with indexed properties).

Yeah, I still think it'd be best to remove both @@species and this.constructor logic, for all the reasons you said here.