Urigo / angular-meteor

Angular and Meteor - The perfect stack
https://www.angular-meteor.com/
MIT License
2.36k stars 622 forks source link

collections are not updated correctly in reactive contexts #656

Closed kongaraju closed 8 years ago

kongaraju commented 9 years ago

I would like use "track by" to prevent rerendering. but its not working...


$scope.tasks= $meteor.collection(function() {
        var query = {sort: { createdAt: -1 }};
          query.limit=Session.get(limit_Key); 

           var tasks= Tasks.find({}, query);

        return tasks;
      }).subscribe('tasks');

html

<div ng-repeat="task in tasks track by task._id" >

I am updating the session item to increase the limit, its regenerating the whole list since track by is not working.

am doing wrong anywhere?

http://stackoverflow.com/questions/32443688/angular-meteor-track-by-not-working

Urigo commented 9 years ago

Can you please share a repo with the publication as well?

kongaraju commented 9 years ago

Source @ https://github.com/kongaraju/simple-todos-angular Deployed @ http://angularmeteortodostrackby.meteor.com/

Login and add more than 10 todos click on loadmore to see more on each load more it is rerendering the whole list, expected to be render only the next 10 records since we are using track by

Thanks for your quick response and support.

nspangler commented 9 years ago

@kongaraju I have run into a similar problem ( https://github.com/Urigo/angular-meteor/issues/681#issuecomment-141685588) . Have you solved the problem?

jacobdr commented 9 years ago

I don't think that this is an issue of the angular-meteor framework, but rather you trying to use a query on the collection object to handle view re-rendering, which is an anti-pattern in angular. You should be using filters to achieve your purpose.

I have submitted a PR on your repo which I believe fixes your issue.

One additional suggestion: why even impose the "limit" on the Mongo query? Unless you have hundreds of records and are experiencing any performance issues, you should consider not limiting your results in the collection, but rather only in the view. And even then you would limit the returned documents in your publish function as well, so your approach would be incomplete.

@Urigo let's wait to see if we get a response, but otherwise I don't think the issue here is the same as in #681, which I will look into next

netanelgilad commented 9 years ago

@jacobdr points are valid but there is still an issue with angular-meteor here which is caused by the cursor changing.

When you change the limit on the session, it causes a re-computation of the reactive function that returns the cursor. When the cursor changes angular-meteor removes all of the elements from the array and starts observing the new cursor and re-adding all of the documents that fit the new cursor. That is done because we cannot know what documents are still in the cursor and which are not so we need to start fresh. The problem here is that in the time that that happens, a digest cycle also happens. When the digest cycle runs the ngRepeat gets reevaluated and it seems only a partial view of the data that should be in the array (cause it is still updating) and then track by doesn't help because for the ngRepeat it seems like the elements have been removed.

@Urigo I think this means we should think of a way to avoid a digest cycle while updating a cursor (which I am not sure is possible because of how Meteor's Tracker works) or we need to find a new way to update the data array when a cursor changes.

Urigo commented 8 years ago

@kongaraju I'm happy to say that this issue should be resolved when using the new helpers syntax in Angular Meteor 1.3. Please let me know if it's not the case

Ben305 commented 8 years ago

@Urigo I'm sorry, but this is still not working with Angular Meteor 1.3.

I have created a sample project to show the problem. This project contains two controllers, one controller is using Angular Meteor with new helpers syntax to fetch the data, the other controller is only using Angular.

Even though is used, the whole list is regenerated when the helper runs.

You can find the sample here: https://github.com/Ben305/angular-meteor-ngRepeat-sample

ozsay commented 8 years ago

@Ben305 We identified the issue and hopefully will fix it on 1.3.6 final release.

The issue is as @netanelgilad described it and has not been changed in 1.3.

mxmzb commented 8 years ago

Works like a charm since 1.3.6 :)

Ben305 commented 8 years ago

@maximsko Sorry, I'm still having problems with 1.3.6

@ozsay Will this issue be fixed in 1.3.7?

sean-stanley commented 8 years ago

I have a massive collection I am trying to use infinite scrolling with and my entire rendered collection is lost using 1.3.7 when I trigger an increase of limit in my subscribe and helper.

The helper is quite complex do I need a simpler one for the improvements in 1.3.6+ to work?

    products() {
      let q = {price: {$gt: 0} }

      if (this.getReactively('query.category') ) q.category = this.query.category;

      if (this.getReactively('query.producer') ) q.producer = this.query.producer;

      if ( Meteor.userId() ) {
        let favourites, lastOrder;

        if (this.getReactively('query.favourites') || this.getReactively('query.lastOrder')) {
          //favourites = Meteor.users.findOne(Meteor.userId()).profile.favourites;
          favourites = _.pluck(Likes.find({liker: Meteor.userId(), category: 'products'}).fetch(), 'likee');
          lastOrder = Meteor.users.findOne(Meteor.userId()).profile.lastOrder;
          if (this.query.favourites && this.query.lastOrder) {
            q._id = {
              $in: _.union(favourites, lastOrder)
            };
          }
          if (this.query.favourites) {
            q._id = {
              $in: favourites
            };
          }
          if (this.query.lastOrder) {
            q._id = {
              $in: lastOrder
            };
          }
        }
      }

      return Products.find(q, {limit: this.getReactively('limit'), sort: this.getReactively('options.sort') })
    }

I will try using the angular built-in limit to see if that works.

In case it's relevant, I iterate through the list using ng-repeat of an Angular 1.5 component. One component holds the helper and then each product has a component with a one-way data-bind to the parent as per the new components guide from the Angular JS team.

I look forward to a solution being found for this.

sean-stanley commented 8 years ago

Sorry more context needed to get my issue and maybe it should be moved to it's own issue if the group doesn't think it fits well here.

My limit is being changed by a directive I wrote called scrollFire that calls a function when it is scrolled into view.This function call is called outside the digest cycle because calling it in the digest cycle re-renders my this.products as empty for a second.

So when I call my output function from scrollfire

function loadMore() {
    if (this.limit <= Products.find().count()) {
      this.limit += pageSize;
      $timeout(()=>{}, 2000)
    }
  }

This and similar variations all work because the function is called outside the digest cycle and so forcing another digest cycle after the cursor has been re-watched makes ng-repeat behave how I want.

I'd like to find a more precise way of determining when it is safe to call $scope.$apply() or similar though.

Urigo commented 8 years ago

@sean-stanley does that work for you with the latest 1.3.9_2? Please let me know so I could close this

Ben305 commented 8 years ago

For me 1.3.9_2 has solved the problems

Urigo commented 8 years ago

awesome!