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

store.filter doesn't work with promises #1865

Closed
iwarshak opened this issue Apr 10, 2014 · 28 comments
Closed

store.filter doesn't work with promises #1865

iwarshak opened this issue Apr 10, 2014 · 28 comments

Comments

@iwarshak
Copy link

I am using store.filter in my route to load model data. I am using filter and not find so that I can pass query params to the server, and also have the model data be modified/observable.

The problem is that the function you pass in to filter expects a true/false return statement. In my situation, my filter function is dealing with promises and it isn't possible to immediately return true/false.

Here is a relevant sample:

App.CommentsRoute = Ember.Route.extend({
  model: function(params, transition, queryParams)  {
      var post = this.modelFor('post');
      var post_id = post.get('id');
      var comments = this.store.filter('comment', {'post_id':post_id},
          function(comment) {
            // doesn't work because filter expects a boolean to return
            // and I am returning a promise
            // I am using .then because associations are now async
            comment.get('post').then(function(p) {
                  return p.get('id') == post_id;
            });
          }
      );
    }
});
@stefanpenner
Copy link
Member

in theory you can use RSVP.filter as it works with promise filterFN's, although it isn't nicely integrated with ember-data.

@iwarshak
Copy link
Author

Could you tell me more about that? I don't see how I could use that at all in this scenario?

Also, is there a better way to do what I am doing? It seems a bit complex

@GetContented
Copy link

Aw man I'm having a fairly similar set of cirucmstances recently (last night, actually!).

Can't you just do something similar to this? I think store.filter is promise-aware isn't it?

  // model class is 'ExcitingPerson'
  model: function() {
    this.store.find('excitingPerson');
    return this.store.filter(function(item) {
      item.firstName.startsWith("B") && item.isAdmin;
    });
  }

Oh... I undersatnd you now, you're not using find because you can't send query params to the server. Hm... I didn't even know you could do that with filter!

I'm a bit confused, though... shouldn't the callback passed to filter actually received the items after they've been resolved? Why are they unresolved?

@iwarshak
Copy link
Author

@JulianLeviston They are unresolved (as I understand it) because comment.post is an association, and now, in emberdata, associations are all async.

TL;DR Only use get on DataBoundPromises inside of observers or computed properties. Otherwise, always treat them as regular promises and use .then.

@GetContented
Copy link

@iwarshak gotcha! Thanks very much for that.

@GetContented
Copy link

So, something like this, then @stefanpenner ?

  App.CommentsRoute = Ember.Route.extend({
    model: function(params, transition, queryParams)  {
      var post = this.modelFor('post');
      var post_id = post.get('id');
      var filterFunction = function(comment) {
        return comment.get('id') == post_id;
      }
      var commentPromises = this.store.filter('comment', {'post_id':post_id});
      var comments = Ember.RSVP.filter(commentPromises, filterFunction);
      return comments;
    }
  });

@iwarshak
Copy link
Author

@JulianLeviston @stefanpenner I think the nice thing about filter() is that when you add/delete/modify a comment, it will automatically apply that filter to see if that comment belongs in the collection (without refetching it)

@stefanpenner
Copy link
Member

App.CommentsRoute = Ember.Route.extend({
    model: function(params, transition, queryParams)  {
      var post = this.modelFor('post');
      var post_id = post.get('id');
      var filterFunction = function(comment) {
         // promise in the filterFN return
         return comment.get('post').then(function(p) {
           return p.get('id') == post_id;
         });
      }
      var commentPromises = this.store.filter('comment', {'post_id':post_id});
      var comments = Ember.RSVP.filter(commentPromises, filterFunction);
      return comments;
    }
  });

@GetContented
Copy link

@stefanpenner um.... not too sure what point your last comment is making, there? isn't that a bit wrong because filterFunction will receive resolved promises? (bit confuzzled)

@stefanpenner
Copy link
Member

the question from the poster was:

The problem is that the function you pass in to filter expects a true/false return statement. In my situation, my filter function is dealing with promises and it isn't possible to immediately return true/false.

As it turns out RSVP.filter supports this scenario

This might not be ideal though, but does "work"

@GetContented
Copy link

@stefanpenner Apologies! I re-read your code and now understand it properly. Obvious bug in the my code was it was comparing comment id id to outer post_id. Silly me :) Thank you. <3

@iwarshak
Copy link
Author

@stefanpenner I get what RSVP.filter can do, but like you said, it doesn't work with ember-data's filter() method.

Do you think that the filter() method, when invoked a function should be able to handle promises which resolve to a boolean, and not just true/false?

@GetContented
Copy link

@iwarshak doesn't the example he provided work for you? That's using ember-data's filter method... and RSVP.filter, no?

@stefanpenner
Copy link
Member

@iwarshak currently the store's filter method depends on being synchronous. It would be a non-trivial change.

I firmly believe a better approach is possible, likely exploring a RX style form of composeability .

@iwarshak
Copy link
Author

@stefanpenner What is RX?

@iwarshak
Copy link
Author

@stefanpenner One way this problem can be solved for this particular case is to be able to directly access the post_id attribute. Is that possible as a workaround?

@iwarshak
Copy link
Author

@stefanpenner @JulianLeviston Tried your code sample using Ember.RSVP.filter fails with:

TypeError: You must pass an array to all.
    at __exports__.default (http://localhost:3000/assets/winter.js:68601:17)
    at invokeResolver (http://localhost:3000/assets/winter.js:68072:9)
    at new Promise (http://localhost:3000/assets/winter.js:68058:9)
    at Function.all (http://localhost:3000/assets/winter.js:68599:14)
    at all (http://localhost:3000/assets/winter.js:67007:22)
    at Object.filter (http://localhost:3000/assets/winter.js:67501:14)
    at MyApp.PostComments.Ember.Route.extend.model (http://localhost:3000/assets/myapp.js:640:35)
    at apply (http://localhost:3000/assets/winter.js:66014:27)
    at superWrapper [as model] (http://localhost:3000/assets/winter.js:65601:15)
    at EmberObject.extend.deserialize (http://localhost:3000/assets/winter.js:96106:23)

store.filter() returns a DS.PromiseArray, but RSVP.filter() is looking for an Array.

Specifically, it fails here:

        if (!isArray(entries)) {
          throw new TypeError('You must pass an array to all.');
        }

@stefanpenner
Copy link
Member

try

App.CommentsRoute = Ember.Route.extend({
    model: function(params, transition, queryParams)  {
      var post = this.modelFor('post');
      var post_id = post.get('id');
      var filterFunction = function(comment) {
         // promise in the filterFN return
         return comment.get('post').then(function(p) {
           return p.get('id') == post_id;
         });
      }
      return this.store.filter('comment', {'post_id':post_id}).then(function(comments) {
        return  Ember.RSVP.filter(comments, filterFunction);
      });
    }
  });

@iwarshak
Copy link
Author

@stefanpenner Same error, except now the comments passed into Ember.RSVP.filter is a DS.FilteredRecordArray

Can I somehow just get the raw post_id value instead of traversing the relationships?

@stefanpenner
Copy link
Member

comments.toArray()

seems like we may need to explore a baked in solution that isn't insane.

@iwarshak
Copy link
Author

@stefanpenner What about accessing the post_id attribute?

@stefanpenner
Copy link
Member

@iwarshak this sort of help belongs on stack overflow. Please reopen your question there.

@iwarshak
Copy link
Author

@stefanpenner FYI - changing to toArray() fixes the error, but now filterFunction is not invoked.

@stefanpenner
Copy link
Member

@iwarshak correct, hence the "shitty solution". It is a non-trivial solution to have both work right now

@iwarshak
Copy link
Author

@stefanpenner

I was able to work around this by accessing the foreign key object directly.

this.store.filter('comment', {'post_id':post_id},
  function(f) {
      return f.get('data.post.id') == post_id
  }
);

Hopefully the filter() method can be reworked in the future to be able to accept promises, or some other solution. I can see situations where accessing it via .data would not work because you are needing to access objects several layers deep.

@xmejkal
Copy link

xmejkal commented May 21, 2014

Hi, is there any progress on Store.filter accepting promises?

@stefanpenner
Copy link
Member

@xmejkal I don't think we plan to add it any time soon. As this will introduce asynchrony to a currently very sync operation. I would be nervous of introducing it.

@ryanberckmans
Copy link

I found a partial solution that may be of use.

In my example, an Employee may work a Shift so long as they don't work two shifts on the same day. Employee hasMany Shifts async.

 route = Ember.Route.extend  # route for employee/Fred/shift-signup
  model: ->
    employee = this.modelFor 'employee'
    store = this.store
    return store.find('shift').then -> # using emberfire, I found it necessary to find() before filter()
      return store.filter 'shift', (shift) ->
        # Won't work!!! BOOM!
        return shift.get('employee') == null && employee.canWorkShift shift # BOOM, canWorkShift returns a Promise because it uses employee.shifts, an async hasMany relationship
        # Instead we do this
        return shift.get('employee') == null # defer canWorkShift to an observer, see below

Use canWorkShift in an observer:

controller = Ember.ArrayController.extend # controller for employee/Fred/shift-signup
  needs: 'employee'
  availableShifts: []
  computeAvailableShifts: (->
    _this = this
    employee = this.get 'controllers.employee.model'
    Ember.RSVP.filter(this.get('model').toArray(), (shift) -> employee.canWorkShift shift).then (availableShifts) ->
      _this.set 'availableShifts', availableShifts
  ).observes('model.@each', 'controllers.employee.model.shifts.@each').on('init')

availableShifts is displayed in handlebars and is read-only.

I'm able to use Store.filter in conjunction with an observer, simulating filtering with promises. The key limitation is that the final product, availableShifts, is read-only.

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

5 participants