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

Rationalizing Symbol.species #1313

Closed
bakkot opened this issue Sep 25, 2018 · 19 comments
Closed

Rationalizing Symbol.species #1313

bakkot opened this issue Sep 25, 2018 · 19 comments

Comments

@bakkot
Copy link
Contributor

bakkot commented Sep 25, 2018

In light of the committee's decision to reject #1289, and the upcoming set methods proposal (which would be one of the first usages of Symbol.species in the spec which worked for methods invoked on non-subclasses) it would be valuable for the committee to decide what our rationale for Symbol.species is. In particular:

  • What non-normative text should describe it?
  • What criteria should we use to decide when Symbol.species is used, going forward?
  • Maybe other questions?

I've suggested some options below. Please react with 👍 and 👎 to indicate your preferences, and suggest other alternatives if you have any.

cc @wycats @gsathya

@bakkot
Copy link
Contributor Author

bakkot commented Sep 25, 2018

One possible alternative: decide arrays are just weird (as they are anyway, for cross-realm web compat reasons), and we should continue to make new generic methods use Symbol.species even when invoked on things which are not instances of the class on which the original method was defined.

@bakkot
Copy link
Contributor Author

bakkot commented Sep 25, 2018

One possible alternative: decide that Symbol.species is specifically intended for use with subclasses, and introduce a brand check before doing the Symbol.species lookup in new methods. For example, the set methods proposal could introduce a SetSpeciesCreate which performed a brand check to see if the methods' callee is a Set, and if not just produce a new Set rather than looking up the constructor and Symbol.species.

@bakkot
Copy link
Contributor Author

bakkot commented Sep 25, 2018

One possible alternative: decide that Symbol.species is specifically intended for use with subclasses, but we're not going to introduce our own brand checks. Make a non-normative change to the description of Symbol.species making this clear, but don't add brand checks to new methods when they're not otherwise necessary.

We might consider using MDN's description for Symbol.species as a starting point:

The species accessor property allows subclasses to override the default constructor for objects.

@bakkot
Copy link
Contributor Author

bakkot commented Sep 25, 2018

One possible alternative: change the behavior of Array.prototype.{concat, filter, map, slice, splice} to not access Symbol.species even on Array subclasses, and instead always create a new Array. This makes subclassing Array less useful, since you'd likely want to introduce your own versions of these methods, and would be breaking if people are subclassing Array and relying on this behavior.

We could rationalize this by saying that those methods need to have their own magic for cross-realm stuff for web-compat reasons, which is true.

@domenic
Copy link
Member

domenic commented Sep 25, 2018

One possible alternative: consider Symbol.species a legacy mistake, and not use it on any newly-introduced methods. Consider removing it from any existing methods where doing so would be web-compatible.

@bathos
Copy link
Contributor

bathos commented Sep 25, 2018

An example of a case where @@species was more useful than the alternative, for consideration:

I once had a Promise subclass where it was important to spawn ordinary promises (or else they’d have cycled). I could have done this by overriding then and catch. But if I had done that instead of using @@species, this class would have had its API broken by the introduction of finally. I believe one important utility of @@species is that it makes subclasses more likely to be forward compatible.

@domenic
Copy link
Member

domenic commented Sep 25, 2018

But if I had done that instead of using @@species, this class would have had its API broken by the introduction of finally

Nope. catch and finally both delegate to then, so overriding then was sufficient.

@getify
Copy link
Contributor

getify commented Sep 25, 2018

I too have a Promise subclass that benefits from Symbol.species. I hope it doesn't go away.

@bathos
Copy link
Contributor

bathos commented Sep 26, 2018

@domenic It delegates to then, yeah — so why does the finally algorithm call SpeciesConstructor? Sorry, tangential question, but I’m pretty curious now.

I think the point about reducing forward compat risks likely stands even if the example I gave was poor. Do you think that is not the case? Or that it is, but its value doesn’t outweight the cost of @@species?

@domenic
Copy link
Member

domenic commented Sep 26, 2018

finally delegates to SpeciesConstructor because of legacy design mistakes carried forward in the name of consistency.

@bathos
Copy link
Contributor

bathos commented Sep 26, 2018

I gather you feel strongly about this haha! I’d like to understand why you concluded that it was a legacy design mistake though.

(I read through #1289, but it didn’t quite fill in the gap — anybody have a link to prior discussion that would clarify?)

@tabatkins
Copy link

Yeah, it's been unclear to me too; @domenic, I've seen you express this strong opinion against Symbol.species before, but I've never seen you express why you feel this way. It's possible I just missed an earlier explanation?

AWB expressed in #1289 pretty clearly that Symbol.species is definitely intended. The feature as a whole makes a ton of sense to me; otherwise you have to reimplement a ton of Array methods if you try to create an Array-like.

Do you object to the behavior just for non-subclasses of Array? Or do you dislike it in general?

Without this or something like it, how do we deal with Array-likes that can't subclass Array? Is your opinion that such classes should just manually re-implement all of the Array methods, and be updated whenever a new Array method is added?

@domenic
Copy link
Member

domenic commented Sep 28, 2018

otherwise you have to reimplement a ton of Array methods if you try to create an Array-like.

@@species is not usable by non-subclasses, so I don't understand this at all. Species does not help non-subclass arrays avoid reimplementing all Array methods in any way.

@littledan
Copy link
Member

littledan commented Sep 28, 2018

OK, it sounds like there are some uses of Promise subclassing, but it's not clear whether species is necessary to achieve the desired functionality or whether overriding other methods would be sufficient.

Are there known use cases for other classes creating an instance of the subclass from methods through species, e.g., TypedArray, Array or RegExp?

@claudepache
Copy link
Contributor

claudepache commented Sep 28, 2018

We can certainly justify the brand-check in ArraySpeciesCreate() as a way to support the legacy way to apply array methods to non-arrays, casting the result into an array in the process. But I don’t think it is a good reason to extend such brand-check to new stuff; we should rather encourage explicit casting functions à la Array.from().

Another thing that bugs me is that, when SpeciesConstructor() or similar is invoked, a default constructor is provided. I think it is a design error (notwithstanding legacy stuff like arrays), because you can’t really guess what the intended constructor is. For example:

Array.prototype.slice.call(nodeList)

will cast a NodeList instance into an Array object, but

class BetterArray extends Array { }
BetterArray.prototype.slice.call(nodeList)

will confusingly not cast a NodeList instance into a BetterArray object.

More generally, any method whose algorithm contains an explicit reference to its class/constructor is expected to behave surprisingly when inherited by a subclass.

@bakkot
Copy link
Contributor Author

bakkot commented Jan 24, 2021

See further discussion of this topic in https://github.com/tc39/proposal-rm-builtin-subclassing.

@getify
Copy link
Contributor

getify commented Jan 24, 2021

per @bakkot, moved my comments here: tc39/proposal-rm-builtin-subclassing#20

@bakkot

This comment has been minimized.

@bakkot
Copy link
Contributor Author

bakkot commented Nov 27, 2024

Closing as done in tc39/how-we-work#157 (which isn't merged yet but we've at least decided what we're doing, which is what @domenic suggested above).

@bakkot bakkot closed this as completed Nov 27, 2024
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

7 participants