rethinkdb / horizon

Horizon is a realtime, open-source backend for JavaScript apps.
MIT License
6.78k stars 351 forks source link

Allow for .subscribe() to prevent returning initial results #46

Open ha1331 opened 8 years ago

ha1331 commented 8 years ago

I'm figuring out "Fusion" by ducktaping an app that uses jquery fullcalendar. ( it's here: https://rdb.space/cal/ ) While it's not actual app that's supposed to do anything worth using, I've come up with a problem with subscribe returning the initial results.

Problem is, that fullcalendar uses a function to fetch the events for the calendar

this.eventSource = function(start, end, timezone, callback){
        // can't currently use "between() type of functionality with fusion if you filter by Date object
        //this.backEndCal.above({start: new Date(start)}).below({start: new Date(end)})
        this.backEndCal.findAll({calendarId: "test"}).value().then(function(results){
            console.log(results);
            callback(results);
        });
    }

It was suggested to me, that I use the initial value from subscribe() to populate the calendar, but I don't want to do that, because fullcalendar is capable of lazy loading the events and I feel I wouldn't want to have the actual app load every event, just month or two.

What happens when I use the eventSource and subscribe() is, every event gets rendered twice:

exampleofeventrenderedtwice

If it's not possible to prevent getting the initial results from subscribe, I think it's at least not totally obvious that every initial result fires .onAdded()

dalanmiller commented 8 years ago

I was talking with @larkost and @deontologician about this specific use case where having initial_results is not useful.

If I want the last 100 documents sorted by a datetime in descending order. I'll first need to grab this data and then I want to subscribe to all future updates via a .subscribe(...). Right now, I'd get a flood of initial_results from the subscribe which I've already collected. Also doing the same query and then doing a .subscribe(...) I don't believe would be possible.

How hard would it be to provide the option initial_results = false? I think that the default should remain to initial_results = true since we want to push people to think about building their apps with changefeeds versus the traditional batch methods but it doesn't fit every case.

coffeemug commented 8 years ago

I think we should just add include_initial optarg to subscribe that's set to True by default. It would be very easy to do and I think would be very valuable. /cc @Tryneus @deontologician

deontologician commented 8 years ago

I'm not totally opposed, but the thinking was that you'd use a between to initially load only the portion you wanted. So in principle you don't need to subscribe to anything that would give you too many initial results

marshall007 commented 8 years ago

I agree with @deontologician, and I think this is solved in a more obvious way with the observable stuff you've been working on. As you hinted at, you'd tend to want to use query.value() (or subscription.initialVals()) to get initial values and subscription.added() for the new ones, which is pretty intuitive. Exposing server-side optargs into the Fusion client starts to sound like a leaky abstraction.

Tryneus commented 8 years ago

Is there going to be some way to avoid race conditions for a user running query.value() followed by subscribe with no initial values? If changes happen in-between, it could easily break a user's frontend.

It should be possible for users to ignore initial values, but we shouldn't encourage calling query.value() followed by a separate subscribe.

marshall007 commented 8 years ago

@Tryneus very true, maybe we should expose subscription.value() which collects initial values and resolves once we get state: 'ready'? That way if you know you're going to open up a subscription you still have an easy way to get at the initial state.

It's possible to receive change notifications for documents you've already received while the feed is still initializing, correct? So subscription.value() would need to take that into account as well. Doing so would be quite handy, actually, since this is a lot to think about (and get right) from an end-user's perspective.

ha1331 commented 8 years ago

I don't think using between() to only get the results you need will solve the problem for every use case, because you might want to observe changes to larger set than what you want to load. Using the calendar as an example.

I wouldn't want to download every event, but I would want to have a notification to changes made outside of what I'm showing. I would want to get the changes made to events I might not need to load at all. For example I might have a "private" calendar showing, but I would still want to see changes made to group calendar. In this case I would fetch the events for the calendar I'm showing and subscribe to every event of all of my calendars.

I see why you would want to courage people to use the realtime features and I'm all for that. I just think there are some situations where there are valid reasons not to go all in on subscription for everything. If it's decided to not allow not getting the initial results, it would be at least nice if there was a way to ignore them.

dalanmiller commented 8 years ago

Maybe also include the option of having onInitial in the object passed into .subscribe()? Versus having the initial results being fed through onAdded?

ha1331 commented 8 years ago

@dalanmiller having onInitial would be pretty darn nice thing.

coffeemug commented 8 years ago

I think we should strive to allow people to write the same code for initial values and future updates because 95% of the time that's what you're going to want to do (at least at the beginning). Currently we have a synced event, but one thing we can do is pass a second argument to the onAdded callback that's set to true if the value is an initial value, and false otherwise. This way the user doesn't have to care if the value is an initial value or an update if they don't need this information, and can easily check for it if they need it.