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

RSVP filter with promise that resolves to array #395

Closed
miguelcobain opened this issue Jul 8, 2015 · 8 comments
Closed

RSVP filter with promise that resolves to array #395

miguelcobain opened this issue Jul 8, 2015 · 8 comments

Comments

@miguelcobain
Copy link

I think it would be really beneficial if we allow the first argument of filter to be a promise, as well as an array of promises.

For example, this is possible:

RSVP.filter([1, 2, 3, 4, 5, 6], function(v) {
  return new RSVP.Promise(function(resolve) {
    setTimeout(function() {
      return v > 2 ? resolve(true) : resolve(false);
    }, 1000);
  });
}).then(function(r) {
  console.log(r);
});
//outputs [3, 4, 5, 6]

but this isn't:

RSVP.filter(Ember.RSVP.resolve([1, 2, 3, 4, 5, 6]), function(v) {
  return new RSVP.Promise(function(resolve) {
    setTimeout(function() {
      return v > 2 ? resolve(true) : resolve(false);
    }, 1000);
  });
}).then(function(r) {
  console.log(r);
});
//outputs "Error: Array Methods must be provided an Array"

Instead, I need to map to an array of promises:

RSVP.filter([1, 2, 3, 4, 5, 6].map(function(i){ return RSVP.resolve(i); }),
function(v) {
  return new RSVP.Promise(function(resolve) {
    setTimeout(function() {
      return v > 2 ? resolve(true) : resolve(false);
    }, 1000);
  });
}).then(function(r) {
  console.log(r);
});
//outputs [3, 4, 5, 6]

I think the filter function should also take a promise that eventually resolves to an array.
This would prove especially useful to Ember Data users and async relationships. One can just invoke filter as RSVP.filter(model.get('someRelationship'), ...). A simple RSVP.filter(this.store.find(...), ...) would be nice as well.

Of course, in my project I could just create a function that normalizes its arguments and then invokes the regular filter func, but I thought I should report in to make this available in core RSVP.

@miguelcobain miguelcobain changed the title RSVP filter RSVP filter with promise that resolves to array Jul 8, 2015
@stefanpenner
Copy link
Collaborator

This isn't a thing. Because it tries to work in the same spirit as all

@miguelcobain
Copy link
Author

I would argue that filter is a bit higher level, i.e it would be strange to do RSVP.all(aPromiseWithArray) but not RSVP.filter(aPromiseWithArray, ...).
Actually, if I'd want the current behavior I could write:

RSVP.filter(RSVP.all(array), ...)

Much nicer than (supposing array is resolved before):

RSVP.filter(array.map(function(i){ return RSVP.resolve(i); }), ...)

even if arrow functions make this a little prettier:

RSVP.filter(array.map(i => RSVP.resolve(i)), ...)

I see filter as a "promisified" version of the regular array filter. With my suggestion, filter would accept both arguments as promises.

The bottom line is that I think filter (and possibly others) should also handle the simplest scenario which is a single promise. Converting from an array of promises to a single promise is already done by all.

However, as I said, it isn't difficult to write a solution yourself. Feel free to close the issue if you still think this is a non-issue.

@stefanpenner
Copy link
Collaborator

RSVP.filter(array.map(i => RSVP.resolve(i)), ...)

I'm not sure why you need this, this is the current behavior.
the first example demonstrates this.

@miguelcobain
Copy link
Author

Oh, yes, you're right.

The thing is that this doesn't work:

this.store.find('model').then(models => {
  return Ember.RSVP.filter(models, ...);
})

Fails with Error: Array Methods must be provided an Array, possibly because RSVP's isArray isn't aware of Ember enumerables?

@stefanpenner
Copy link
Collaborator

Fails with Error: Array Methods must be provided an Array, possibly because RSVP's isArray isn't aware of Ember enumerables?

correct, sometime soon enumerables will become ES2015 @@iterables and RSVP Does still need to support those.

work around:

this.store.find('model').then(models => {
  return Ember.RSVP.filter(models.toArray(), ...);
});

@miguelcobain
Copy link
Author

Great. Thank you. Sorry for my misunderstanding.

@stefanpenner
Copy link
Collaborator

Nice. Sorry for my misunderstanding.

no problem, we should make the switch to iterables sometime during 2.1 or 2.2 of ember

@miguelcobain
Copy link
Author

Along with switching to ES2015 class, decorators, etc. The Ember developer experience will be hugely improved. Can't wait!

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