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

map does not work with iterables other than strings, arrays and plain objects #52

Open
gurpreetatwal opened this issue Jul 27, 2020 · 4 comments

Comments

@gurpreetatwal
Copy link

gurpreetatwal commented Jul 27, 2020

The JSDoc on map indicates that input can be an iterable, however it doesn't seem to work with other objects that implement the iterable protocol including built-in ones like Set.

Test Case

const P = require('blend-promise-utils');

const main = async function () {
  const numbers = new Set([1, 2, 3]);
  await P.map(numbers, function (number) {
    console.log(number);
  });
};

main();
Expected output
1
2
3
Actual output
@gurpreetatwal
Copy link
Author

gurpreetatwal commented Jul 27, 2020

Two potential resolutions:

  1. Edit the JSDoc to remove iterable and replace it with string
  2. Edit the code to add support for iterable

My vote is for resolution two for the following reasons:

  1. personal bias 😜 -- modifying map to support all Iterables fixes my problem
  2. users might assume that the function supports iterables and the behavior as implemented causes the code to fail silently leading to errors that are hard to debug
  3. supporting Iterables "feels" correct (which is most likely why the JSDoc calls out)

More than happy to help out with a PR for either of those or any other solutions :)

@ftrimble
Copy link
Collaborator

Do you have an idea for maintaining the same input typing for the output as the input if we support generic iterables? Generally I don't see an easy way to achieve that, which is why I'd probably rather update the docs.

@gurpreetatwal
Copy link
Author

By "input typing for the output" do you mean the return value for the map function itself or for the input into the iteratee function? Just want to make sure we're looking at the same thing

@ftrimble
Copy link
Collaborator

i.e. the return type for map is the same as the input type

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

2 participants