Infinite loops result if arguments to method calls evaluate differently on each run of the function about meteor-reactive-method HOT 20 CLOSED

This is definitely a fixable bug, I'll fix it and add a test!

from meteor-reactive-method.

I also agree that there should be more documentation about cases that could result in too many reruns that should be avoided. It would be useful to have a variant of Tracker.autorun that limits how often the function can rerun, to have a blunt hammer to deal with these sorts of situations when they are hard to resolve.

from meteor-reactive-method.

As a side note, do you have any ideas for alternate names for Tracker.autorun ? I proposed Meteor.run before, emphasizing the fact that it is run the first time (which was non-obvious to me for a while). I've also considered Meteor.register to emphasize the other side. In any case I feel the method name itself could convey more meaning, and may as well be exported from the Meteor object, since it's at least as fundamental as subscribe and publish.

from meteor-reactive-method.

No, sorry, I don't have any ideas on that topic. I think Meteor.run is too vague and similar to Meteor.call. Meteor.register sounds like a callback or something.

from meteor-reactive-method.

I saw your DevShop talk. Cool. I see why Tracker replaces event emitters. For one-time deferred values, I feel the caution Travker requires you to have with the enclosing function is dangerous, and not very discoverable, but I still want to play with this project some more..

Do you think this issue of matching up the call with its return value between autoruns is fixable? I'm stumped, when the arguments may not identically evaluated every re-run. I should write a test case even if I can't think of how to fix it.

from meteor-reactive-method.

Hey I've created a fork of this- check out my ideas there, and I'd love your further input.

from meteor-reactive-method.

@chicagogrooves I replied to your Crater comment also. Where is the best place to continue this discussion? I'm not sure that you have actually solved the problem.

from meteor-reactive-method.

Sorry - so many places to check for updates now, Meteor is growing so fast :) We can talk here or in my repo, or a Slack room you'd prefer, send me an invite (my github username at gmail).. I just updated the README in my repo to explain the current behavior, and the tests there cover my use cases.

from meteor-reactive-method.

OK let's just keep talking here.

Basically my concern is that you've "thrown the baby out with the bath water" in some sense. It looks like in your version of the package the method is only ever called once, even if the arguments change. What if you want to call the same method with different arguments, for example if you use the helper arguments in your method call?

from meteor-reactive-method.

So where is that use case in the test cases ? If you want to 'bring the baby back', I suspect the way you'll have to do is by requiring an additional argument, a "handle", as the creator of Reactive Promise called it, for the sole purpose of identifying which method call to associate results with. This should allow arguments to change at will. https://github.com/arsnebula/reactive-promise/

from meteor-reactive-method.

Yep, adding a handle would be a good idea if people want to do that part explicitly. That's a great solution!

from meteor-reactive-method.

pinging @Slava on this..

from meteor-reactive-method.

Try this package instead: https://atmospherejs.com/mnmtanish/call

This package supports using a "unique id" for each method call, which will stop it from rerunning when the arguments change. The whole point of reactive-method is to rerun the method when its arguments change. I'm going to close this issue because I disagree with this change, and update the README to warn people about infinite loops and changing arguments.

from meteor-reactive-method.

Ok, I figured as much, and updating the README will help.

from meteor-reactive-method.

FYI My new alternate implementation doesn't "throw the baby out the window" (as humorous as that sounds). Your input ? https://atmospherejs.com/deanius/promise

from meteor-reactive-method.

@deanius this is really really awesome! Does it replace all of the use cases of my package? If so, I'd be happy to officially deprecate mine and point people to yours.

from meteor-reactive-method.

I think it would, if it's fully tested, wanna help :) ? So far I think I've addressed 1) taking in parameters to the helper 2) handling multiple instances on the page 3) not being sensitive to the arguments of the Meteor.call..

A few issues remain 1) what should it show while waiting - the string 'waiting...' is just temporary

from meteor-reactive-method.

Oh that's interesting. I guess you want to be able to distinguish between the result being null, and the promise not having returned yet. Maybe return an object you can === like PromiseHelper.Loading? Then you can do like {{#if PromiseHelper.isLoading}}XXX{{/if}}

from meteor-reactive-method.

OK - this package is ready for prime-time.

Note the change of name from PromiseHelper to ReactivePromise, since it's usable outside of a helper context. It also no longer monkey-patches Meteor.call - Meteor.promise is recommended like before.

I added arguments for loadingText, and failure text, if the promise is rejected.. I also tested that it works for synchronous methods, so it could be a default behavior (eventually) for Template.xx.helpers to wrap its helper methods in case they return Promises. (To be able to mix sync/async would be the goal)

If you want to move people over to my package (and this discussion), that'd be great. Just let me know if/when you're going to do that. Thanks for the help so far!

from meteor-reactive-method.

@deanius I might not have time today, but I'll try to find time this week. Thanks for building this - the reactive method package here was meant mostly as a proof of concept, so I'm happy to not have to maintain it anymore.

from meteor-reactive-method.