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

tracking: support iterables in @std/collections APIs #5470

Open
10 tasks
iuioiua opened this issue Jul 17, 2024 · 3 comments
Open
10 tasks

tracking: support iterables in @std/collections APIs #5470

iuioiua opened this issue Jul 17, 2024 · 3 comments
Labels
collections good first issue Good for newcomers PR welcome A pull request for this issue would be welcome

Comments

@iuioiua
Copy link
Collaborator

iuioiua commented Jul 17, 2024

Following on from #4671, it seems there are APIs within @std/collections that could support Iterables rather than just arrays. From a quick scan, these symbols might be able to be tweaked to support iterables:

  • chunk()
  • dropLastWhile()
  • dropWhile()
  • intersect()
  • sample()
  • slidingWindows()
  • sortBy()
  • takeLastWhile()
  • takeWhile()
  • withoutAll()

See #3390 for an example of how with was done for groupBy().

@Liam-Tait
Copy link
Contributor

A larger change to consider. Should collections use generators? This would be breaking change.

Generators give lazy evaluation, memory efficiency, and sometimes better performance

Example:

function* takeWhile<T>(
  iterable: Iterable<T>,
  predicate: (el: T) => boolean,
): Generator<T> {
  for (const element of iterable) {
    console.log(`takeWhile(): ${element}`);
    if (!predicate(element)) {
      break;
    }
    yield element;
  }
}

function* map<T, U>(
  iterable: Iterable<T>,
  transform: (el: T) => U,
): Generator<U> {
  for (const element of iterable) {
    console.log(`map(): ${element}`);
    yield transform(element);
  }
}

const arr = [1, 2, 3, 4, 5, 6, 7, 8, 9];
const result = takeWhile(map(arr, (n) => n * 100), n => n < 500)
console.log([...result])

@iuioiua
Copy link
Collaborator Author

iuioiua commented Sep 9, 2024

I think we should stick to the current approach unless the performance benefits are sufficient. We'd have to measurements before coming to that decision.

@Liam-Tait
Copy link
Contributor

Liam-Tait commented Sep 9, 2024

The move to iterables means that generators that never return are valid arguments.

  function* counter() {
    let i = 0;
    while (true) {
      yield i++;
    }
  }

Many of these functions would hang

Examples:

takeWhile(counter(), v === 0) // predicate will never be true
chunk(counter(), 10)
dropLastWhile(counter, v => v > 30)
dropWhile(counter, v => v === 0) // predicate will never be true

Here are some options:

  1. Don't handle the case, onus is on the user to know the generator is infinite. Likely case is throwing an Uncaught RangeError: Invalid array length when the result array is too long
  2. Catch the error and rethrow?
  3. Have a max iterations option available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections good first issue Good for newcomers PR welcome A pull request for this issue would be welcome
Projects
None yet
Development

No branches or pull requests

2 participants