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

As an app developer I want to know when syncing is done #732

Closed
xMartin opened this issue Jun 23, 2014 · 28 comments
Closed

As an app developer I want to know when syncing is done #732

xMartin opened this issue Jun 23, 2014 · 28 comments
Labels

Comments

@xMartin
Copy link
Member

xMartin commented Jun 23, 2014

When syncing documents or initially loading documents there's single events for each new or updated document. If I use these events to update the UI lots of updates are made in a short time causing things to flicker or jump around in case of updated order in a list or something and it is also just inefficient. I would like to know when a sync cycle or an initial loading is done so I can use that event to trigger one single update of the UI with all the new data.

@michielbdejong
Copy link
Member

Fixed by #698 - listen for:

remoteStorage.local.on('local-events-done', function() {
  console.log('all local events have been fired now');
});

@xMartin
Copy link
Member Author

xMartin commented Jun 24, 2014

Nice! Why is this part of local? Is it also available in no-chache mode/build? Docs? How does it relate to "wire-done"?

@raucao
Copy link
Member

raucao commented Jun 24, 2014

There's no way to know when "syncing is done", which is why the event is now called "wire-done" instead of "sync-done". The local-events-done just means that during startup all local change events for the data that is already stored have been fired.

@xMartin
Copy link
Member Author

xMartin commented Jun 24, 2014

True, does not what I am looking for.

@michielbdejong
Copy link
Member

Right, I see what you mean. There is no explicit event for "sync done at least once", only for "this sync cycle has completed successfully". It's the 'done' event on remoteStorage.sync. It's what is used to set the timer for the next sync cycle. So using that, you could do something like:

remoteStorage.local.on('local-events-done', function() {
  var syncDoneOnce = false;
  console.log('all local events have been fired; waiting for initial sync to finish now');
  remoteStorage.sync.on('done', function() {
    if (!syncDoneOnce) {
      syncDoneOnce = true;
      console.log('now you can safely render your view the first time');
      renderTheView();
      console.log('from now on, will re-render the view after each incoming change');
      remoteStorage.on('change', renderTheView);
    }
  });
});

@michielbdejong
Copy link
Member

Actually we should add this to some sort of FAQ. Reopening this with a 'docs' label.

@raucao
Copy link
Member

raucao commented Jun 25, 2014

Hacking it this way doesn't make any sense in my opinion. It's the whole reason why we only fulfill the promise of getAll and getObject if the cached data isn't out of date and has been synced recently.

@xMartin You can now safely use getAll and if the objects are older than 2 sync cycles, it will fetch the objects first and then fulfill the promise. I have it working like a charm with a nice loading route during the promise wait in the Webmarks master.

@michielbdejong
Copy link
Member

right, that would be an alternative. Put the getAll call into your renderTheView function, and then don't register any change handlers until it has completed once. So you could do:

var changeHandlerRegistered = false;
function renderTheView() {
  remoteStorage.stuff.getAllStuff().then(function(allStuff) {
    updateHtml(allStuff);
    if (!changeHandlerRegistered) {
      remoteStorage.stuff.onChange(renderTheView);
      changeHandlerRegistered = true;
    }
  });
}

renderTheView();//called once on page load

@raucao
Copy link
Member

raucao commented Jun 25, 2014

This is very complicated with not much benefit as well. There won't be any change events until things actually change remotely. We register those before even doing the getAll in Webmarks and it works like a charm.

@michielbdejong
Copy link
Member

@skddc Under caching strategy 'ALL', there will be 'remote'-origin change events during initial sync, as the local store is filling up with data it's fetching from the remote store. Also, you would get all the 'local'-origin events, don't you?

@raucao
Copy link
Member

raucao commented Jun 25, 2014

  1. Then why does it work so well in Webmarks?
  2. You obviously ignore those (if not disable them via config), when you don't need them: https://github.com/skddc/webmarks/blob/master/app/routes/application.js#L14

@michielbdejong
Copy link
Member

  1. if you're not getting 'remote'-origin change events during initial sync (i.e., just after OAuth, when the local store is entirely empty) then probably you are using the 'SEEN' caching strategy, not the 'ALL' caching strategy.
  2. right, that would work. Let's document that as a third option, in addition to As an app developer I want to know when syncing is done #732 (comment) and As an app developer I want to know when syncing is done #732 (comment), with the restriction that it will only work under 'SEEN' and 'FLUSH' strategies.

@raucao
Copy link
Member

raucao commented Jun 25, 2014

@michielbdejong
Copy link
Member

And you're absolutely sure that no change events are received during initial sync? Can you add a debug statement on line https://github.com/skddc/webmarks/blob/master/app/routes/application.js#L15?

@raucao
Copy link
Member

raucao commented Jun 25, 2014

No, I'm not. Thinking about it, it would be wrong if there weren't any. But it doesn't matter anyway, as we're setting the model to the getAll response when loading the bookmarks list view and then only objects from new change events after that content-loading are added to the in-memory collection. This way it doesn't matter what happens beforehand, and you don't have to think about or care about any of it. It just always works correctly.

@michielbdejong
Copy link
Member

So you're not fixing the OP's problem, then, which was:

If I use these events to update the UI lots of updates are made in a short time

You need an (Ember-independent) mechanism that suppresses those small updates until initial sync is complete.

@raucao
Copy link
Member

raucao commented Jun 25, 2014

So you're not fixing the OP's problem, then, which was:

If I use these events to update the UI lots of updates are made in a short time

Yes, because you're not using the events in order to load the initial data. It is still the reason we fixed the getAll behaviour and the intuitive way to go for app developers, as almost any other data persistence library gives you a call like that by default.

You need an (Ember-independent) mechanism that suppresses those small updates until initial sync is complete.

This has nothing at all to do with Ember! Exactly zero. It's just loading the objects into your collection that you're rendering, which you would do with any framework and in any normal app implementation.

@raucao
Copy link
Member

raucao commented Jun 25, 2014

By the way, from your comments I get the feeling that you're only implementing example apps with next to no code, especially not models, collections, bindings, etc.. That is a very different point of view and it doesn't help people using an app framework like Angular, Ember, Marionette, React and whatnot, as they usually do something very similar to what I posted as an example and don't even have to worry about the problems you have without these concepts.

@michielbdejong
Copy link
Member

I don't understand the distinction between

just loading the objects into your collection that you're rendering

and its opposite,

using the events in order to load the initial data.

I understand you make a distinction between loading data "into the collection" and displaying data on the screen, but I don't know enough about mvc to understand it, sorry.

I'll just create a FAQ entry based on the first two solution, and then you can add the third option which in #732 (comment) you say is simpler.

@raucao
Copy link
Member

raucao commented Jun 25, 2014

Ok.

@xMartin
Copy link
Member Author

xMartin commented Jun 25, 2014

I wasn't looking for an "at least once" thing. To me it looks like remoteStorage.sync's "done" event is what I was looking for. Is there any docs?

Here's some (pseudo)code that shows what my idea is. Does this make sense?

remoteStorage.myModule.privateClient.cache("")

remoteStorage.on("features-loaded", function(){

    remoteStorage.myModule.privateClient.getAll("mypath/").then(function(data){

        var dirty = false

        // store and views are not bound in any way
        store.setData(data)
        views.update()

        remoteStorage.myModule.privateClient.on("change", function(event){
            if(event.newValue && event.oldValue){
                console.log(event.path + " was updated")
            }else if(event.newValue){
                console.log(event.path + " was created")
                store.put(event.newValue)
                dirty = true
            }else if(event.oldValue){
                console.log(event.path + " was deleted")
                store.remove(event.oldValue.id)
                dirty = true
            }
        })

        remoteStorage.sync.on("done", function(){
            if(dirty){
                views.update()
                dirty = false
            }
        })
    })
})

@michielbdejong
Copy link
Member

Right, that makes sense,yes. Although you really want to use 'ready' instead of 'features-loaded' if you're precise about it. I'll add that example as well.

@xMartin
Copy link
Member Author

xMartin commented Jun 26, 2014

Ok. The current event docs are not very helpful ("features-loaded: fired when all features are loaded"). Where do you add it? Where are these FAQs?

@michielbdejong
Copy link
Member

They don't exist yet, I'm doing a doc sprint the week of 7 July, then I'll create them. a bit like howto-guides. Either in the starter-kit, or on remotestorage.io, or both.

@michielbdejong
Copy link
Member

I also noticed that the local-events-done event is not in http://remotestorage.io/doc/code/index/Events.html (we actually don't have any documentation at all for remoteStorage.local, we should add that to src/cachinglayer.js).

@raucao
Copy link
Member

raucao commented Nov 13, 2016

FYI: when remoteStorage is ready, there's remoteStorage.sync.on('done'). This will fire, when all single sync tasks have been completed and a new sync is queued. There's also req-done for the single tasks.

I think we should probably emit that event on the remoteStorage instance itself as well, in order for it to be a proper public event. Then again, maybe it's also nice to keep these things namespaced on the classes. Not sure. Any opinions?

@raucao
Copy link
Member

raucao commented Dec 5, 2017

In 1.0.x, there is now an event sync-req-done and sync-done for both the single sync operations as well as a whole cycle completing (i.e. everything has been synced according to the caching config): https://remotestoragejs.readthedocs.io/en/latest/js-api/remotestorage.html#list-of-events

Also, the startSync() function will return a promise, which resolves after a successful sync. However, it's recommended to use the events, as the library automatically syncs changes, both on app startup as well as periodically after that.

@raucao raucao closed this as completed Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants