Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Should arrays be created through Symbol.species? #4

Closed
bakkot opened this issue Jul 15, 2021 · 32 comments
Closed

Should arrays be created through Symbol.species? #4

bakkot opened this issue Jul 15, 2021 · 32 comments

Comments

@bakkot
Copy link
Contributor

bakkot commented Jul 15, 2021

Continuing discussion from here:

Should the arrays created to hold the sublists be created using ArraySpeciesCreate, so that if you subclass Array and call groupBy the resulting object will have values which are instances of your subclass?

I lean towards no. The existing uses of ArraySpeciesCreate are all straightforward transformations of the original array: you put an array (as this) in, you get an array out. By contrast, this method returns an object (which holds some number of arrays). I don't think it's obvious that ArraySpeciesCreate is appropriate here.

@jridgewell
Copy link
Member

I just added https://github.com/tc39/proposal-array-grouping/pull/5 PR (because apparently %TypedArray.prototype% has everything that Array.prototype does – minus flatMap?), which uses a similar TypedArraySpeciesCreate. Eg, when using a Uint8Array, I definitely expect my subarrays to be Uint8Arrays too.

So I think this is a similar case, but definitely something we should discuss.

@bmeck
Copy link
Member

bmeck commented Jul 20, 2021

I think the key difference might be in if you expect user subtypes to override normal builtin types when doing this. I do know @syg was attempting to do stuff to species but don't know the current state of things.

@zloirock
Copy link
Contributor

All Array and %TypedArray% methods are designed to be subclassable. .filter returns an instance of the same class that initial array, the rest Array methods do the same. groupBy is an extended version of .filter that just split the initial array into groups - so here should be used @@species - I don't see any reason to doubt it.

@ljharb
Copy link
Member

ljharb commented Jul 20, 2021

I could just as easily see an argument that since groupBy does not itself return an array, that species need not be used.

Since the general feeling in committee is that species was a mistake, and that modulo web compat it would be removed, it seems useful to minimize new uses of it.

@zloirock
Copy link
Contributor

@ljharb it's a very strange argumentation - as the result of filtering, I'd expect to get instances of the same class as the initial array.

Until the committee does not propose an alternative method of instance methods subclassing, this should be used. The current usage of this and other kinds of subclassing can't be removed due to compatibility reasons. Subclassing is a very important part of the language and it can't be removed just because someone is a bad engineer and he doesn't want to optimize it. @@species is bad for any other builtins, but it was designed mainly for arrays - and here it's the best option at this moment.

@ljharb
Copy link
Member

ljharb commented Jul 20, 2021

The implication that browser implementors are bad engineers is unhelpful and inappropriate.

Subclassing builtins is most often a poor design choice, so I don't agree it's an important part of the language, but it's not going to be helpful to debate those kinds of opinions here.

@zloirock
Copy link
Contributor

Subclassing builtins is most often a poor design choice, so I don't agree it's an important part of the language

Bad language design is to add some fundamental concept to the language standard, and then after a few years trying to get rid of it; add new methods like the existing ones, with behavior different from the old in similar cases making them not intuitive (holes, thisArg, now subclassing), etc.

The implication that browser implementors are bad engineers is unhelpful and inappropriate.

However, it's a fact. I don't know how you are, but I face this every day. Moreover, otherwise, such proposals would not have been received.

@theScottyJam
Copy link

If only we were all omniscient beings who could see the end from the beginning - then we could get it right the first time. I guess that makes us all bad engineers :).

@zloirock
Copy link
Contributor

Don't be confused, these are 2 different (however, in places overlapping) problems - bad language design and lazy implementers -)

@syg
Copy link

syg commented Jul 20, 2021

I would prefer new methods to not honor @@species at all.

@theScottyJam
Copy link

theScottyJam commented Jul 20, 2021

I guess this all boils down to "when should species be used, and when shouldn't they be used"? If we can find a clear way to define that, then we should be able to answer this question.

For example, if we say that species should only be used if the return value is a normal array, then what do we do with the proposed .partition() function? (it doesn't matter if we actually add .partition() or not, we're just trying to figure out how to apply species in different scenarios). Would the outer array be from the species and the inner array be normal? That's a little weird.

Honestly, I think the most consistent way to apply species is to have all new, returned arrays be constructed using the species.

But, I also agree that this species thing is a weird feature that probably shouldn't be used. But, since we've got it, we might as well honor it the best we can, otherwise, we'll just create more inconsistency in the language.

@syg
Copy link

syg commented Jul 20, 2021

But, since we've got it, we might as well honor it the best we can, otherwise, we'll just create more inconsistency in the language.

I disagree. I think the bar should be higher in JS, because it's a language that has backwards compat in perpetuity. Consistency here is another thing to be weighed, and for @@species I wouldn't consider consistency compelling in itself.

@theScottyJam
Copy link

theScottyJam commented Jul 20, 2021

I'm not sure I really understand where you're coming from. @@species doesn't affect anyone unless someone's actually trying to use it. And if someone's trying to use it, we might as well provide them with a consistent experience (at least as much as possible).

I agree that code that's relying on it is inherently more fragile and it should be considered an anti-pattern to use @@species, but I don't agree that we should give up on it and ask everyone who's using it to stop. Is there any particular motive we have to stop supporting this feature, causing code that relies on it to not function properly with any new methods, and effectively rendering @@species useless? Unless there's some sort of performance issue involved with @@species that we're wanting to start avoiding, or unless the committee is actually thinking about deprecating this feature, I can't think of any reason why we wouldn't just provide support for it anyways.

@syg
Copy link

syg commented Jul 20, 2021

I'm not sure I really understand where you're coming from. @@species doesn't affect anyone unless someone's actually trying to use it.

I'm coming from an implementation maintainability, performance, and security point of view, where @@species has significant, and I would argue disproportionate, downside given how rarely it's useful. More on that is said here: https://github.com/tc39/proposal-rm-builtin-subclassing

@zloirock
Copy link
Contributor

...and, as I already wrote above, why this proposal can't be applied in the current form - here.

@ljharb
Copy link
Member

ljharb commented Jul 20, 2021

Those web compat constraints don't apply to new code, though.

@bterlson
Copy link
Member

@zloirock insults and uncharitable generalizations are a counter-productive way to advocate for your ideas. Please opt for a more kind and professional approach in the future.

@zloirock
Copy link
Contributor

@ljharb however, it's a proposal removing subclassing from the existent features, not a recommendation about avoiding it in the future. I don't see any final decisions about those kinds of subclassing.

@zloirock

This comment has been minimized.

@bterlson

This comment has been minimized.

@zloirock

This comment has been minimized.

@bterlson

This comment has been minimized.

@zloirock
Copy link
Contributor

@bterlson sorry, but I will not choose other words in cases like this since it's hypocrisy and, accordingly, violation of other, more important, rules.

@bathos
Copy link

bathos commented Jul 20, 2021

I think the fact that it doesn't return an array but rather an object that collects values in arrays makes the species question moot? It's providing buckets of values and this is the model used to represent that - I don't see how that implies a special relationship to the type of array from which it was called.

It seems more akin to a regexp match result/groups object than the result of a chain-friendly species-honoring method.

(I tend to weigh consistency's value higher than many people, but really what that's about is wanting to avoid the confusion/complexity of new obscure quirky exceptions to things that would otherwise be rules. I don't think this will create a quirky exception because there's really no precedent for this on Array or its other species-infected pals.)

@ljharb
Copy link
Member

ljharb commented Jul 21, 2021

I really like the regex match object comparison; i think that’s a very compelling clincher.

@zloirock
Copy link
Contributor

zloirock commented Jul 21, 2021

@bathos so, in your opinion, for typed arrays it's also just buckets and should be used usual arrays?

It's near to results of some .filter calling, moreover - this proposal is a child of array filtering proposal. The resulting null prototype object is near to the .groups object on the result of regex .exec calling, but definitely, the resulting arrays are not related to .exec or .match results that are not just simple arrays and have some specific properties like .input, .index and already mentioned .groups.

@bathos
Copy link

bathos commented Jul 21, 2021

@bathos so, in your opinion, for typed arrays it's also just buckets and should be used usual arrays?

Yes, though I have trouble picturing a scenario where this would be useful in either form with typed arrays.

@zloirock
Copy link
Contributor

zloirock commented Jul 22, 2021

A simple example where it should be the same constructor instance is the case when one of the results assigned the initial variable:

const { left, process } = list.groupBy(fn);
list = left;
for (const element of process) { /* ... */ }

It could be used everywhere, a simple example - shopping cart in the store - we could buy only selected or available items. This shopping cart could be a subclass of Array for use Array.prototype methods and own logic:

class ShoppingCart extends Array { /* ... */ }

The same about typed arrays - we could split them into partitions, handle one and left another. Why typed arrays? As always - speed and memory.

@zloirock
Copy link
Contributor

So, any feedback on the previous comment?

@bathos
Copy link

bathos commented Jul 26, 2021

I understand your conclusion. It makes sense given a particular interpretation of what the method “does” and what its result represents. That part, which doesn’t currently have a concrete answer, seems to be where we disagree.

If the method does end up having a complement on %TypedArray.prototype% that buckets using TypedArray instances of the same kind as the instance on which the method was called, I would end up agreeing with your interpretation, though, since at that point the pattern has been codified unambiguously. Ignoring @@species in that case creates a pretty nasty have-your-cake-and-eat-it-too problem; I’d hope the object is either “a generic grouping result” or it’s species-sensitive. Something in between is a bit evil. :)

This is why I asked about TypedArray use cases. You said:

Why typed arrays? As always - speed and memory.

Those are benefits TypedArrays bring to their use cases. I’m curious if there are known use cases specifically for “grouping” elements of TypedArrays. On the surface this seems odd to me because the order and relative position of those elements is very often inherently part of their meaning, unlike in most ordinary arrays. I think seeing a real-ish example use case would help show whether the property values in the groups object are logically also TypedArrays.

@zloirock
Copy link
Contributor

That part, which doesn’t currently have a concrete answer...
If the method does end up having a complement on %TypedArray.prototype%...

This part is currently in the spec draft.

On the surface this seems odd to me because the order and relative position of those elements is very often inherently part of their meaning, unlike in most ordinary arrays.

TypedArrays could be used and used anywhere where used arrays of numbers, it's not something magic or specific.

I’m curious if there are known use cases specifically for “grouping” elements of TypedArrays.

Grouping is required for any divide-and-conquer algorithm, many different divide-and-conquer algorithms are useful for arrays of numbers -)

@jridgewell
Copy link
Member

Closing as the committee agreed to remove species support from the methods.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants