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

std / collections #978

Closed
wants to merge 8 commits into from
Closed

std / collections #978

wants to merge 8 commits into from

Conversation

LionC
Copy link
Contributor

@LionC LionC commented Jun 17, 2021

This is the proposal to add an std/collections module, which has been discussed on Discord and in this Github Discussion. It contains a spec, horrible jsdoc descriptions and an okay-ish example for every function.

The module and it's terminology are heavily inspired by the kotlin standard collections library and the respective (very similiar) non-mutating lodash parts, which are both battle tested and see a lot of use.

There are still a lot of TODOS:

  • Freeze list of function
  • Finalize typings (some may not be narrow enough right now)
  • Improve documentation, as it is horrible right now and just meant for internal discussion
  • Implement and fix examples
  • Add tests

I moved the discussion into a draft PR as proposed in the linked Discussion by @kt3k , because Discussions do not seem to be set up well for large files and continues changes on the spec / the implementation code.

I am looking to finalize the list of functions soon and am asking for input on that, given the recently added examples that have been requested.

@CLAassistant
Copy link

CLAassistant commented Jun 17, 2021

CLA assistant check
All committers have signed the CLA.

@jsejcksn
Copy link
Contributor

@LionC asked me to move my comment here from the discussion:

On the topic of syntactic sugar functions:

Any time syntax sugar aligns with your needs, it feels very welcome indeed.

However, an individual's vocabulary/grammar as well as their programming experience plays a huge role in this. Unrelated to those subjective aspects, both maintainability and performance are also crucial considerations.

From a performance perspective: while calling a function is cheap, it's not free. There's definitely something to be said about providing highly performant functions, but ones that merely rename a concise combination of built-ins would be better left to the user to implement IMO.

On the other hand, it can be very useful to provide a consistent grammar. As an example, using includes vs contains to indicate whether an item is found in a collection:

@wperron said:

I think we should be conservative in what we include in this module if it is accepted.

There is a precedent of removal of std functions which have only provided nominal syntax sugar. Here is one example: denoland/deno#7059. Searching the PR/commit history will surface many others.

@kt3k
Copy link
Member

kt3k commented Jun 22, 2021

@LionC Sorry for the delayed response. I took a look at each API and I'm currently in favor of adding 43 of them, and against adding 17 of them.

Among proposed 60 APIs, I found the following 43 APIs useful:

// Array predicates (3)
declare function includesAll<T, O extends T>(collection: Array<T>, needles: Array<O>): boolean
declare function includesAny<T, O extends T>(collection: Array<T>, needles: Array<O>): boolean
declare function includesNone<T, O extends T>(collection: Array<T>, needles: Array<O>): boolean

// Array transformations to Array (12)
declare function mapNotNullish<T, O>(collection: Array<T>, transformer: Selector<T, O>): Array<NonNullable<O>>
declare function takeFirstWhile<T>(collection: Array<T>, predicate: Predicate<T>): Array<T>
declare function takeLastWhile<T>(collection: Array<T>, predicate: Predicate<T>): Array<T>
declare function dropFirstWhile<T>(collection: Array<T>, predicate: Predicate<T>): Array<T>
declare function dropLastWhile<T>(collection: Array<T>, predicate: Predicate<T>): Array<T>
declare function distinct<T>(collection: Array<T>): Array<T>
declare function distinctBy<T>(collection: Array<T>, selector: Selector<T>): Array<T>
declare function sortBy<T>(collection: Array<T>, selector: Selector<T, number | string>): Array<T>
declare function intersect<T>(collection: Array<T>, withCollection: Array<T>): Array<T>
declare function union<T>(collection: Array<T>, withCollection: Array<T>): Array<T>
declare function without<T>(collection: Array<T>, toRemove: T): Array<T>
declare function withoutAll<T>(collection: Array<T>, toRemove: Array<T>): Array<T>

// Array transformations to value (10)
declare function firstNotNullishOf<T, O>(collection: Array<T>, selector: Selector<T, O>): NonNullable<O> | undefined
declare function findLast<T>(collection: Array<T>, predicate: Predicate<T>): T | undefined
// overlaps with https://github.com/tc39/proposal-array-find-from-last though
declare function sumOf<T>(collection: Array<T>, selector: Selector<T, number>): number
declare function maxBy<T>(collection: Array<T>, selector: Selector<T, number | string>): T | undefined
declare function minBy<T>(collection: Array<T>, selector: Selector<T, number | string>): T | undefined
declare function maxWith<T>(collection: Array<T>, comparator: (a: T, b: T) => number): T | undefined
declare function minWith<T>(collection: Array<T>, comparator: (a: T, b: T) => number): T | undefined
declare function maxOf<T, O extends number | string>(collection: Array<T>, selector: Selector<T, O>): O | undefined
declare function minOf<T, O extends number | string>(collection: Array<T>, selector: Selector<T, O>): O | undefined
declare function average(collection: Array<number>): number | undefined

// Array transformations to other formats (8)
declare function zip<T, U>(collection: Array<T>, withCollection: Array<U>): Array<[ T, U ]>
declare function unzip<T, U>(pairs: Array<[ T, U ]>): [ Array<T>, Array<U> ]
declare function associateBy<T>(collection: Array<T>, selector: Selector<T, string>): Record<string, T>
declare function groupBy<T>(collection: Array<T>, selector: Selector<T, string>): Grouping<T>
declare function partition<T>(collection: Array<T>, predicate: Predicate<T>): [ Array<T>, Array<T> ]
declare function chunked<T>(collection: Array<T>, size: number): Array<Array<T>>
declare function permutations<T>(collection: Array<T>, size?: number): Array<Array<T>>
declare function windowed<T>(collection: Array<T>, size: number, config?: { step?: number, partial?: boolean }): Array<Array<T>>

// Record Predicates (1)
declare function includesValue<T>(collection: Record<string, T>, value: T): boolean

// Record Transformations to Records (7)
declare function filterEntries<T>(collection: Record<string, T>, predicate: Predicate<[string, T]>): Record<string, T>
declare function filterKeys<T>(collection: Record<string, T>, predicate: Predicate<string>): Record<string, T>
declare function filterValues<T>(collection: Record<string, T>, predicate: Predicate<T>): Record<string, T>
declare function filterValuesNotNullish<T>(collection: Record<string, T>): Record<string, NonNullable<T>>
declare function mapEntries<T, O>(collection: Record<string, T>, transformer: Selector<[string, T], [string, O]>): Record<string, O>
declare function mapKeys<T>(collection: Record<string, T>, transformer: Selector<string, string>): Record<string, T>
declare function mapValues<T, O>(collection: Record<string, T>, transformer: Selector<T, O>): Record<string, O>

// Groups (2)
declare function reduceGroups<T, A>(collection: Grouping<T>, reducer: (accumulator: A, current: T) => A, initialValue: A): Record<string, A>
declare function aggregateGroups<T, A>(collection: Grouping<T>, aggregator: (accumulator: A, current: T, key: string, first: boolean) => A, initalValue: A): Record<string, A>

Especially Array transformations to other formats and Record Transformations to Records categories look very useful. These look common and non-trivial tasks. So I believe adding these APIs would actually solve the users' problems and add practical values to the standard modules.

On the other hand, the following 16 APIs look too trivial to include in the standard modules because these can be implemented too easily with JS builtin APIs and also because there seem no performance gains by providing these.

// Array predicates (3)
declare function none<T>(collection: Array<T>, predicate: Predicate<T>): boolean
// seems better to write `arr.every((x) => !predicate)`
declare function isEmpty(collection: Array<unknown>): boolean
// seems better to write `arr.length === 0`
declare function isNotEmpty(collection: Array<unknown>): boolean
// seems better to write `arr.length > 0`

// Array transformations to Array (7)
declare function takeFirst<T>(collection: Array<T>, n: number): Array<T>
// seems better to write `arr.slice(0, n)`
declare function takeLast<T>(collection: Array<T>, n: number): Array<T>
// seems better to write `arr.slice(arr.length - n)`
declare function dropFirst<T>(collection: Array<T>, n: number): Array<T>
// seems better to write `arr.slice(n)`
declare function dropLast<T>(collection: Array<T>, n: number): Array<T>
// seems better to write `arr.slice(0, arr.length - n)`
declare function filterNot<T>(collection: Array<T>, predicate: Predicate<T>): Array<T>
// seems better to write `arr.filter((x) => !predicate(x))`
declare function filterNotNullish<T>(collection: Array<T>): Array<NonNullable<T>>
// seems better to write `arr.filter((x) => x)`
declare function runningReduce<T, A>(collection: Array<T>, reducer: (accumulator: A, current: T) => A, initialValue: A): Array<A>
// seems better to write `arr.reduce(reducer, initalValue)`

// Array transformations to value (4)
declare function lastIndex(collection: Array<unknown>): number
// seems better to write `arr.length - 1`
declare function first<T>(collection: Array<T>): T | undefined
// seems better to write `arr[0]`
declare function last<T>(collection: Array<T>): T | undefined
// seems better to write `arr[arr.length - 1]` or `arr.at(-1)`
declare function single<T>(collection: Array<T>, predicate: Predicate<T>): T | undefined
// seems better to write `arr.find(predicate)`

// Record Predicates (1)
declare function includesKey<T>(collection: Record<string, T>, value: string): boolean
// seems better to write `record.hasOwnProperty(value)`

// Groups (1)
declare function countGroups(collection: Grouping<unknown>): Record<string, number>
// seems redundant if we have mapValues (`mapValues(group, (x) => x.length)`)

And lastly, I'm against including deepMerge API because there seems no way to give a correct type definition to this API.

@nayeemrmn
Copy link
Contributor

@kt3k I would also exclude the following:

//    includesAll(collection, needles)
// => needles.every((v) => collection.includes(v))
declare function includesAll<T, O extends T>(collection: Array<T>, needles: Array<O>): boolean
//    includesAny(collection, needles)
// => needles.some((v) => collection.includes(v))
declare function includesAny<T, O extends T>(collection: Array<T>, needles: Array<O>): boolean
// This is a negation of `includesAny()`...
declare function includesNone<T, O extends T>(collection: Array<T>, needles: Array<O>): boolean
//    mapNotNullish(collection, transformer)
// => collection.filter((v) => v != null).map(transformer)
declare function mapNotNullish<T, O>(collection: Array<T>, transformer: Selector<T, O>): Array<NonNullable<O>>
//    without(collection, toRemove)
// => collection.filter((v) => v == toRemove)
declare function without<T>(collection: Array<T>, toRemove: T): Array<T>
//    withoutAll(collection, toRemove)
// => collection.filter((v) => toRemove.includes(v))
declare function withoutAll<T>(collection: Array<T>, toRemove: Array<T>): Array<T>

If these one-liners are deemed too long, we're essentially saying that higher-order Array methods with even the smallest of arrow functions shouldn't be seen in application code.

In addition we should consider that functions like intersect(), union() are more suited to Sets and not intuitively defined for arrays.

@jsejcksn
Copy link
Contributor

@kt3k

declare function filterNotNullish<T>(collection: Array<T>): Array<NonNullable<T>>
// seems better to write `arr.filter((x) => x)`

The example in the commit doesn't make the implementation completely clear here, but your suggestion is different than my interpretation. Based on the return type using the utility NonNullable<T>, I interpreted "not nullish" to mean that the filtered values would be not null or undefined, rather than any falsy value (like 0 or an empty string). @LionC Will you clarify?

declare function last<T>(collection: Array<T>): T | undefined
// seems better to write `arr[arr.length - 1]` or `arr.at(-1)`

Thanks for mentioning this stage 3 proposal method: arr.at(n). I didn't know about it before.

@kt3k
Copy link
Member

kt3k commented Jun 23, 2021

@nayeemrmn Ok. I agree with your suggestion of exclusion of includesAll, includesAny, includesNone, without, and withoutAll.

For mapNotNullish, I think it is not collection.filter((v) => v != null).map(transformer), but collection.map(transformer).filter((v) => v != null), and I think we can provide more memory-efficient version of this by skipping creating the intermediate array collection.map(transformer). If the source array is very large and the result array is small, that memory efficiency would matter.

In addition we should consider that functions like intersect(), union() are more suited to Sets and not intuitively defined for arrays.

I partly agree with this. What do you think if these APIs return Set<T>?

declare function intersect<T>(collection: Array<T>, withCollection: Array<T>): Set<T>
declare function union<T>(collection: Array<T>, withCollection: Array<T>): Set<T>

Or maybe better to accept sets as parameters as well?

declare function intersect<T>(collection: Array<T> | Set<T>, withCollection: Array<T> | Set<T>): Set<T>
declare function union<T>(collection: Array<T> | Set<T>, withCollection: Array<T> | Set<T>): Set<T>

BTW, Nayeem, do you agree with addition of the other 37 APIs?

@jsejcksn I agree with you. filterNotNullish should be arr.filter((x) => x != null), but I still think this is a little too trivial to include in the standard modules.

Thanks for mentioning this stage 3 proposal method: arr.at(n). I didn't know about it before.

Yup, we should definitely keep the higher stage proposals in mind to avoid the duplication in the future.

@wperron
Copy link
Contributor

wperron commented Jun 23, 2021

@kt3k I think we can (and should) be more conservative than that. I agree with @nayeemrmn but I think we can remove even more from this proposal. I made a quick gist showing quick and easy alternatives to some of the proposed functions. I'd keep the following functions:

// Array transformations to Array (12)
declare function distinct<T>(collection: Array<T>): Array<T>
declare function distinctBy<T>(collection: Array<T>, selector: Selector<T>): Array<T>
declare function intersect<T>(collection: Array<T>, withCollection: Array<T>): Array<T>
declare function union<T>(collection: Array<T>, withCollection: Array<T>): Array<T>

// Array transformations to value (10)
declare function findLast<T>(collection: Array<T>, predicate: Predicate<T>): T | undefined
// overlaps with https://github.com/tc39/proposal-array-find-from-last though

// Array transformations to other formats (8)
declare function zip<T, U>(collection: Array<T>, withCollection: Array<U>): Array<[ T, U ]>
declare function unzip<T, U>(pairs: Array<[ T, U ]>): [ Array<T>, Array<U> ]
declare function groupBy<T>(collection: Array<T>, selector: Selector<T, string>): Grouping<T>
declare function partition<T>(collection: Array<T>, predicate: Predicate<T>): [ Array<T>, Array<T> ]
declare function chunked<T>(collection: Array<T>, size: number): Array<Array<T>>
declare function permutations<T>(collection: Array<T>, size?: number): Array<Array<T>>

// Record Transformations to Records (7)
declare function filterEntries<T>(collection: Record<string, T>, predicate: Predicate<[string, T]>): Record<string, T>
declare function filterKeys<T>(collection: Record<string, T>, predicate: Predicate<string>): Record<string, T>
declare function filterValues<T>(collection: Record<string, T>, predicate: Predicate<T>): Record<string, T>
declare function mapEntries<T, O>(collection: Record<string, T>, transformer: Selector<[string, T], [string, O]>): Record<string, O>
declare function mapKeys<T>(collection: Record<string, T>, transformer: Selector<string, string>): Record<string, T>
declare function mapValues<T, O>(collection: Record<string, T>, transformer: Selector<T, O>): Record<string, O>

I think the first 3 groups clearly add something to the standard library by implementing functions that are not-so-trivial yet are pretty common. They also don't have anything close to them in the ECMAScript spec at the moment, and I believe nothing in the proposals either (with the exception of findFromLast but we can always just change the underlying implementation for that function once it's landed, or simply deprecate it)

The last group of functions (filterEntries, filterKeys, filterValues, mapEntries, mapKeys and mapValues) could probably be implemented using Object.entries, but getting it to return a shally copy of the original with some modifications is less trivial and can easily grow to a couple of lines of code, I think they'd be quite useful.

There's a few functions I'm on the fence about as well:

  • windowed I understand what it does now, but I don't understand the use case for it. It's good to know that it exists but in the absence of real-world example where it could be used, I move to remove it from the proposal
  • includesValue The provided example seems pretty straight forward, but I'm worried about its behavior with more deeply nested data structures.
  • filterValuesNotNullish I don't think it provides meaningful value that filterValues from the proposal doesn't already bring
  • reduceGroups and aggregateGroups I'm on the fence about these, in fact, I'm on the fence about this Grouping interface. I'm not sure it really adds anything to the type signature, and after pruning a lot of the functions from the proposal, it's only used in one of the return types. These two functions seem like they would be very dependent on the shape of the objects being reduced/aggregated. For now I'd leave those out, maybe we could bring them back in at a later time.

@LionC
Copy link
Contributor Author

LionC commented Jun 23, 2021

Ok a lot of new comments - thanks everyone for the tremendous amount of input! I am really happy to see that there is interest in this and I will try and add my personal reasoning and mindset for all of the mentioned points here.

Clarifications

Some functions seem to be misunderstood (the examples provided in the spec might be unclear, please feel free to point that out!):

filterNotNullish

@jsejcksn

Based on the return type using the utility NonNullable<T>, I interpreted "not nullish" to mean that the filtered values would be not null or undefined, rather than any falsy value (like 0 or an empty string). @LionC Will you clarify?

Yes this is correct, nullish means null | undefined - I stole the term from the nullish coalescing operator, which is part of the spec now. This is also relevant for @kt3k s and @wperron s examples, which both assume otherwise by filtering out 0 and false which is almost never what you want and leads to annoying bugs when it happens.

single

@ktek posted an example equating single to find, while single will only return a value if and only if there is exactly one match for the given predicate - it is equal to something like somArray.filter(predicate).length === 1 ? someArray.find(predicate) : undefined.

runningReduce

@kt3k posted an example equating this to a normal reduce, while the result of runningReduce actually looks quite different. One could argue that you can simply transform any reducer(acc, cur) to (acc, cur) => [ ...acc, reducer(acc[acc.length - 1], cur) ] to achieve a runningReduce, but that adds a lot of mental parsing overhead to an already hard to parse reduce call for a special case that is quite simple - runningReduce allows you to use any reducer you have lying around without modification.

takeLastWhile

@wperron posted an equivalent in their gist that suggests this is equal to original.slice(original.findIndex((i) => {}) - original.length), while takeLastWhile starts from the end of the array and stops as soon as it finds the first false result for the given predicate. This means the only way to do this using findIndex would be to .reverse() the array - twice.

firstNotNullishOf

@wperron posted an equivalent in their gist that equals this to .filter(nullishPredicate)[0], while firstNotNullishOf (like all -Of functions) actually applies the given selector to each element until it finds the first non-nullish result. This can only be achieved by chaining .map(selector) with .filter(nullishPredicate) and an access [0] - with all the performance implications.

associateBy

@wperron posted an equivalent in their gist that equals this to .map(it => ({ [selector(it)]: it }), while associateBy actually returns an object, so the equivalent would be something like Object.fromEntries(array.map(it => [ selector(it), it ])).

On the topic of trivial-ish functions

I would like to separate the mentioned "trivial" functions into two categories before talking about them:

Two (and more) Liners

I personally do not consider things that need to be expressed by two or more chained function calls one-liners. I do not claim this to be common wisdom, but I think it is a really good idea to only do one thing per line in code that is meant to be read and maintained by a group of people, especially if those "one things" are iterating through a whole array and applying some function.

So I will call those functions two liners for the rest of the post.

One Liners

By one liners, I mean functions that would intuitively and naively be implemented with one function call natively, with that function call not being a reduce (because I think you very rarely reduce without it being multiline, as reducing is just complex to parse mentally if if has any complexity at all, which it usually does).

Performance

For two-liners, I want to emphasize @kt3k s point from above:

I think we can provide more memory-efficient version of this by skipping creating the intermediate array

If the naive implementation would be to chain two Array methods, then the std/collections version will provide a performance boost by iterating through the array only once, saving time and memory, besides any other case specific optimizations that can be done because the function is narrower than the general version.

For one-liners, I must say that I personally do not think performance is a significant argument to include or exclude a function as suggested by @jsejcksn

From a performance perspective: while calling a function is cheap, it's not free. There's definitely something to be said about providing highly performant functions (...)

I think it is hard to actually have an even measurable performance difference between e.g. filterNotNullish(foo) and foo.filter(it => it !== undefined && it !== null). And I would guess that if your use case actually cares about that difference, you are probably not using an stdlib in general.

Expressiveness

One of the purposes of this module, I would argue of any function in general, is making code express what it is doing to humans reading it. I mean, this is one of the core purposes of source code in general :-)

For two-liners, the difference in expressiveness is really big in my opinion - the collections functions express one thing that we as humans want to achieve and think of as "one thing" in the scope we are using it in. Chaining two or more function calls with multiple expressions to achieve that one thing works, but it needs to be mentally parsed every single time it is read to figure out what the author actually wanted to achieve.

I really want to emphasize this - in all the discussions around this module I am missing this argument. I fell like everyone seems to only want to include functions that the average user will not know how to implement. I think this is very flawed. I doubt that most of us only use other peoples code if we do not know how to implement it.

Besides just not wanting to spend the time implementing something (including optimizing it, fixing corner cases etc.), external functions give us a grammar of sorts to express what we are doing. It gives us a tool to tell the reader what we are doing without making them play everything through their head to figure out what is happening and without them, or us respectively, needing to know how it is exactly implemented.

There is a reason that lodash (which includes pretty much all of the functions in this module) and the kotlin std lib include even the one-liners we are talking about here while being very popular - it is a common grammar that people understand and that makes code easier to read, focussing attention on the actually complex things going on.

Expresiveness is of course not a binary term but a sliding scale so to speak. Some people might think that isEmpty(arr) is exactly as expressive as arr.length === 0, while others might say that the second one reads less fluently. Some people might even say that arr.reduce((accumulator, current) => accumulator += current) / arr.length is as expressive as average(arr). I do not claim to have the one correct line to draw here, but I am missing the relevance of expressiveness in the whole discussion.

-Not vs !

From the suggestions to remove certain -Not functions, I understand that people think that ! is totally fine. While kotlin (which inspired this) seems to have arrived at the conclusion that reading -Not(somePredicate) is better than parsing a small ! which hugely changes the meaning of the expression, I can totally understand that not-ing with a ! is a very common concept and should be used.

On Groupings

@wperron

reduceGroups and aggregateGroups I'm on the fence about these, in fact, I'm on the fence about this Grouping interface. I'm not sure it really adds anything to the type signature, and after pruning a lot of the functions from the proposal, it's only used in one of the return types. These two functions seem like they would be very dependent on the shape of the objects being reduced/aggregated. For now I'd leave those out, maybe we could bring them back in at a later time.

I first want to clarify, that as stated in the PR, the type signatures are not final, especially regarding the aliases, which I just added to simplify the spec discussion. I would actually vote for completely removing aliases for the actual public implementation, as the typescript LSP doesn't resolve them on showing hover information..

The reason I included specific Grouping functions (and I would guess the reason kotlin has) is that this is just a very common use case. A lot of every-day random cases as well as a huge number ob algorithms need to group things by some property at some point, making the { [x]: Array<Something> } data structure, which looks arbirtarily random, a common one. Treating each group in that data structure separately while keeping the structure itself is, in my experience, usually what happens, hence the functions above.

On Set-ish functions

@nayeemrmn

In addition we should consider that functions like intersect(), union() are more suited to Sets and not intuitively defined for arrays.

While I agree on kind of an abstract level, the need for those functions comes up a lot in all kinds of situations. And almost everytime it comes up, the data structures at hand are Arrays and the data structure needed in the end is also an Array (to serialize, to pass to something else etc.). While mathematically, intersections and unions happen on Sets, I think the semantics on Arrays stated in the examples is pretty intuitive and also match those of e.g. lodash.

On deepMerge

I really think we should find some version of this to include. The deepmerge package on npm has 16 million downloads per week, almost every single big package depends on it. I think this is madness. At it's core, it really just merges objects and arrays, having some fancy (and rarely used) optional behavioural options.

The need to deepMerge comes up a lot more than one might think in my experience, especially working with tree-like structures (think object filters, nested errors etc). The current solution of most people seems to be to rely on a third party to provide that function for them, even though it could be a standard 10-liner. It is just complex enough for people to be too lazy to do it.

@kt3k said

And lastly, I'm against including deepMerge API because there seems no way to give a correct type definition to this API.

I do not think removing things that are hard to type is an option, as (apparent by its popularity), people will deepMerge anyways, just with some other dependency or their own (bad) implementation. The typings in the proposal are pretty much the ones npm/deepmerge uses and I think they are fine for most use cases. They are not perfect, as deepMerge just walks the line of what is possible with the typescript type system right now, but we will not prevent people from needing it and using it by not providing it.

Proposed changes

Given everything I said above and all the arguments posted (again thanks a lot to everyone for investing their time and effort), I want to propose to include the following functions:

  • The ones @kt3k mentioned in his first post
    • Excluding includesAll, includesAny, includesNone but keeping without and withoutAll, as I really see the expressiveness argument for them - you are not filtering, you are removing things - just look at the amount of results when searching "how to remove an element from an array"
    • Includng runningReduce and single, as they seem to have been misunderstood and are actually a bit more complex
    • Including filterNotNullish - the case for this function comes up a lot and the naive implementation that I see all the time (.filter(it => it) can lead to bugs with falsey values that are annoying to find. The fact that two separate people in this discussion have posted that implementation, even though it behaves differently, supports the reason for its existence in my opinion. It also plays a lot nicer with typescript, as it automatically returns the desired type without (verbosely) explicitly marking the type guard in the filter call.

Conclusion

I will remove the functions that I excluded in my proposed changed changes above from the spec, as I seem to be the only one fighting for them and even I have conceded them :-)

For the rest, I am looking forward to more exchange and would welcome pointers on bad examples, as there seemed to be some misunderstandings with the ones provided.

I am really looking forward to hopefully see this thing happen - thanks again to everyone who is participating and for reading this crazy long post.

@kt3k
Copy link
Member

kt3k commented Jun 30, 2021

Because this proposal has so many items and so much information already, I suggest we'd split this into 2 parts. First part includes less controversial 17 APIs: distinct, distinctBy, intersect, union, findLast, zip, unzip, groupBy, partition, chunked, permutations, filterEntries, filterKeys, filterValues, mapEntries, mapKeys, mapValues, and the 2nd part includes the others.

I'll create a draft PR for the first part.

@ry
Copy link
Member

ry commented Jul 15, 2021

Landed in #993

Please open new PRs for modifications

@ry ry closed this Jul 15, 2021
@kt3k kt3k mentioned this pull request Jul 27, 2021
26 tasks
@LionC LionC deleted the module-collections branch August 23, 2021 10:54
@LionC LionC mentioned this pull request Aug 27, 2021
44 tasks
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

Successfully merging this pull request may close these issues.

7 participants