{reactive:false} doesn't work #771

Closed
Tarang opened this Issue Mar 3, 2013 · 6 comments

Projects

None yet

5 participants

@Tarang
Contributor
Tarang commented Mar 3, 2013

I was able to reproduce this myself, using the leaderboard example I changed one line:

Template.leaderboard.players = function () {
    return Players.find({}, {sort: {score: -1, name: 1}, reactive: false});
};

But the behavior is not as expected. In the leaderboard example it is still reactive.

Additionally using:

Template.leaderboard.preserve([".leaderboard"])

Did not work either, even in conjunction with {reactive:false}

Also on SO: http://stackoverflow.com/questions/15187465/setting-reactive-false-when-searching-a-collection-in-meteor-still-updates-th

@glasser
Member
glasser commented Mar 4, 2013

I think the issue is that reactive: false is processed inside Minimongo to decide whether or not to do a little "observe and if it changes, invalidate" thing. But #each has its own separate observeChanges call which uses observe directly, not reactivity, to update changes.

In deftemplate.js, this code decides whether to use observe-based smart #each or just a non-updating #each:

  // if arg isn't an observable (like LocalCollection.Cursor),
  // don't use this reactive implementation of #each.
  if (!(arg && 'observeChanges' in arg))
    return orig.call(this, arg, options);

We could change that to (!(arg && 'observeChanges' in arg && arg.reactive)), I guess, though that would make the interface to templating more complex...

@belisarius222

@dglasser I'm not sure that change would work by itself:

from handlebars/evaluate.js:

'each': function (data, options) {


 var parentData = this;
 if (data && data.length > 0)
   return _.map(data, function(x, i) {
     ...

else

  return Spark.labelBranch(
    'else',
    ...

If the #each argument is a cursor, then this code will always render the
else block. I guess maybe that's what you meant by "make the interface
to templating more complex".

On Mon, Mar 4, 2013 at 12:22 AM, David Glasser notifications@github.comwrote:

I think the issue is that reactive: false is processed inside Minimongo
to decide whether or not to do a little "observe and if it changes,
invalidate" thing. But #each has its own separate observeChanges call
which uses observe directly, not reactivity, to update changes.

In deftemplate.js, this code decides whether to use observe-based smart
#each or just a non-updating #each:

// if arg isn't an observable (like LocalCollection.Cursor),
// don't use this reactive implementation of #each.
if (!(arg && 'observeChanges' in arg))
return orig.call(this, arg, options);

We could change that to (!(arg && 'observeChanges' in arg &&
arg.reactive)), I guess, though that would make the interface to
templating more complex...


Reply to this email directly or view it on GitHubhttps://github.com/meteor/meteor/issues/771#issuecomment-14364429
.

@trymbill
trymbill commented Mar 4, 2013

The work around here is then to append .fetch() to the return, so that the template doesn't get a cursor, but just the array.

return Players.find({}, {sort: {score: -1, name: 1}, reactive: false}).fetch();

Tried that and it worked fine.

@glasser
Member
glasser commented Mar 5, 2013

Good point Ted.

And yeah, I should have mentioned that fetch() is the standard way to do this, thanks @trymbill ! But I understand why it would be surprising that reactive: false doesn't work.

@gschmidt
Member
gschmidt commented Mar 5, 2013

Nice catch @Tarangp.

@glasser and I discussed and we agree that this is confusing -- we think the best solution would be to make observe()/observeChanges() only deliver the initial document set when passed a cursor with {reactive: false}. This would make your code do what you expected without having to also call .fetch().

@glasser is going to take this (and add a note to the docs for observe()/observeChanges().)

@glasser
Member
glasser commented Mar 5, 2013

Should be fixed on devel!

@glasser glasser closed this Mar 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment