pouchdb-community / ember-pouch

PouchDB/CouchDB adapter for Ember Data
Apache License 2.0
281 stars 76 forks source link

Site Requires Refresh to Work #121

Open srsgores opened 8 years ago

srsgores commented 8 years ago

I have created a site using ember-pouch, but the site fails to load the first time the user navigates to it.

Here is the link

The first time you navigate you just see a loading spinner; but after 1 refresh, the site works as intended. Is there perhaps an issue with the pouchdb database not being initialized early enough?

Since the site uses appcache, try loading it in incognito (CTRL + SHIFT + N).

I am using cloudant, and ember-pouch 3.1.1, so I'm not sure if that's related.

broerse commented 8 years ago

I use cloudant for http://bloggr.exmer.com/ together with https://www.npmjs.com/package/ember-cli-deploy-couchdb and it works fine. Perhaps we can debug this?

srsgores commented 8 years ago

Good to know. @broerse, if you try loading the site in incognito mode or in an unused browser, you'll see that the the site fails to load the first time.

broerse commented 8 years ago

Yes I saw that. Can I look at some code? I am not an expert but perhaps I find the problem anyway :smile:

srsgores commented 8 years ago

@broerse, I sent you an invite to the repository. Let me know if you find anything.

broerse commented 8 years ago

I don't seem to have the invite. Sent it again?

srsgores commented 8 years ago

Sent again

broerse commented 8 years ago

It was just slow this time. Got it

srsgores commented 8 years ago

Did you get it?

broerse commented 8 years ago

Changing your adapter to this will get your nav-bar in. Error not solved. Still looking.

import config from "../config/environment";

import PouchDB from "pouchdb";
import {Adapter} from "ember-pouch";

let remote = new PouchDB(config.db.url + "/" + config.db.name);
let db = new PouchDB(config.db.name);

db.sync(remote, {
  live: true,   // do a live, ongoing sync
  retry: true   // retry if the conection is lost
});

export default Adapter.extend({
  db: db,

    // Change watcher for ember-data
  immediatelyLoadAllChangedRecords: function() {
    this.db.changes({
      since: 'now',
      live: true,
      returnDocs: false
    }).on('change', function (change) {
      var obj = this.db.rel.parseDocID(change.id);
      // skip changes for non-relational_pouch docs. E.g., design docs.
      if (!obj.type || obj.type === '') { return; }

      var store = getOwner(this).lookup('service:store');
      store.findAll(obj.type);
    }.bind(this));
  }.on('init')
});
broerse commented 8 years ago

On http://localhost:4200/blog/posts it works with the changed adapter.

srsgores commented 8 years ago

@broerse, yes that page does, but none of the other pages do. Good start. Noticed a new error:

ember.debug.js:31563 Error: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing. at Error (native) at exports.openTransactionSafely (http://localhost:4200/assets/vendor.js:71813:16) at allDocsQuery (http://localhost:4200/assets/vendor.js:71011:21) at allDocs (http://localhost:4200/assets/vendor.js:71109:5) at idbAllDocs (http://localhost:4200/assets/vendor.js:71112:3) at PouchDB.idb_allDocs as _allDocs at http://localhost:4200/assets/vendor.js:69105:11 at http://localhost:4200/assets/vendor.js:69104:12 at Array.map (native) at allDocsKeysQuery (http://localhost:4200/assets/vendor.js:69099:27)onerrorDefault @ ember.debug.js:31563 ember.debug.js:31563 Error: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.

broerse commented 8 years ago

Interesting. That is indexDB. Is it not something in your browser? Don't have time now to check if I also see this error.

dweremeichik commented 8 years ago

@srsgores briefly looking at your site, i noticed an error in the console regarding accessing a property of an undefined variable, not sure if this is causing you trouble.

Also, are you using error templates in addition to loading templates? In my issue, I do not get stuck on a loading template if it can kick over to the error template when it hits an error (This only happens to me on findRecord because it rejects, findAll resolves with essentially an empty set.

Per the route that you linked in, are you using findAll or findRecord?

Be careful using the adapter that @broerse linked in, see issue #50 (the whole thing is a good read). And note that even if you only have a few documents, if you delete and add, or make revisions over time the update seq (revision history) grows (which will slow your app down no matter what adapter you use). As of right now, ember pouch does not wait for initialization to complete to return your findAll / findRecord promise which is why the adapter that @broerse linked in is so appealing (which I believe still won't fix the issue for findRecord).

broerse commented 8 years ago

From ember-pouch release 3.2.0 you can now write this:

import config from "../config/environment";

import PouchDB from "pouchdb";
import {Adapter} from "ember-pouch";

let remote = new PouchDB(config.db.url + "/" + config.db.name);
let db = new PouchDB(config.db.name);

db.sync(remote, {
  live: true,   // do a live, ongoing sync
  retry: true   // retry if the conection is lost
});

export default Adapter.extend({
  db: db,

  unloadedDocumentChanged: function(obj) {
    let store = this.get('store');
    let recordTypeName = this.getRecordTypeName(store.modelFor(obj.type));
    this.get('db').rel.find(recordTypeName, obj.id).then(function(doc) {
      store.pushPayload(recordTypeName, doc);
    });
  }
});

Still looking for the real problem.

srsgores commented 8 years ago

@dweremeichik, I'm using findAll, then doing a this.store.filter, as the current adapter doesn't support findQuery.

dweremeichik commented 8 years ago

@srsgores so something like this:

items: new Ember.RSVP.Promise((resolve) => {
        this.store.findAll('item').then((items) => {
          resolve(items.filterBy('type', 'food').sortBy('name'));
        });
      })

In my app, this resolves the page, and doesn't get stuck on the loading screen (it still doesn't get the data, but it does not get stuck either.)

The page I was asking about was: leveraging-computed-properties-in-emberjs. Seems odd to be using a findAll there, is that being use for something other than loading the blog post itself?

srsgores commented 8 years ago

@dweremeichik, I'll give your promise solution a whirl, and post back if it works.

The reason I can't use findQuery is, once again, because the adapter, as it is right now, does not support it; internally, the only solution would involve pouchdb-find, and filtering all local content. Couchdb does have views, but as of right now, there's no way to configure them for use with the current adapter.

dweremeichik commented 8 years ago

@srsgores yeah, I am aware of the state of the adapter. Hence the promise solution i gave you (i use it).

Again, because i don't know what your code is doing , i was just curious as to why you would need findAll to load a single result when you could simply leverage the id and use a findRecord instead. However, if we are both experiencing the same issue findRecord will provide you no solace any ways .

For what it is worth, I believe that relational-pouch is really close to having findQuery implemented.

srsgores commented 8 years ago

@dweremeichik, the reason is that my blog uses slugs, and not ids, so I wouldn't have access to the post/:post_id in my route. For reference, this is what I had earlier:

model(params) {
        let post = null;
        return this.store.findAll("post").then(() => {
            return this.store.filter("post", (post) => {
                return post.get("slug") === params.post_slug;
            }).then((resolvedPosts) => {
                return resolvedPosts.get("firstObject");
            });
        });
    }
srsgores commented 8 years ago

@dweremeichik, I tried your solution, but still got the same result. The page still needs a refresh:

model(params) {
        let post = null;
        return new Ember.RSVP.Promise((resolve) => {
            return this.store.findAll("post").then(() => {
                return this.store.filter("post", (post) => {
                    return post.get("slug") === params.post_slug;
                }).then((resolvedPosts) => {
                    return resolve(resolvedPosts.get("firstObject"));
                });
            });
        });
    }
dweremeichik commented 8 years ago

@srsgores it was more of to see if your loading template would resolve to the page (even with no data).

As far as the slugs go, you should be able to save them as the id (not entirely sure how it works with ember-pouch) as relational pouch supports it. It might be worth seeing if you can set _id in a model. If it works i will gladly update the documentation to reflect it as it is kind of lacking.

Also i wonder if that get error i was seeing in the console was from resolvedPosts. get("firstObject") as resolvedPosts (I believe) would be null.

srsgores commented 8 years ago

@dweremeichik, no it wasn't. I double-checked, and it was from my model title property for ember-cli-document-title.

dweremeichik commented 8 years ago

So, I believe that I have found a solution to this issue, in the blog of @nolanlawson of all places :). The explanation of what is going on here more or less, can be found here. The solution is here. I have looked into multiple ways of fixing this previously, and by far this appears to be the most elegant. The problem is, we don't have access to those "low level" pouchdb functions in this plugin, as that is managed by relational pouch. It seems that places like findAll, findRecord and all of the others in this plugin might be the place to fix this.

It is my understanding that findRecord throws an error when no data is found, while findAll returns an empty set. I'm not sure how the others behave either, but it would mean that when we see the error or empty set, we give the remote/fallback database a try.

broerse commented 8 years ago

@dweremeichik @srsgores FYI 4.0.1 now supports query and queryRecord

dweremeichik commented 8 years ago

@broerse Awesome, I know that one has been kicking around for a while; glad to see it is finally fixed. I'm not really sure how that helps here. For reference see the initial post in this issue. Also see #123, an issue that I created and then later closed in favor of this one. If you also reference my previous post with the links to nolanlawson's blog you'll see how he overcomes the same issue that @srsgores is encountering; only nolanlawson is doing it in a "pure" pouchdb environment (no ember).

From the tests I did months ago on this issue, the loading screen that @srsgores mentions in his first post is caused by having a loading route but no error route, therefore ember will sit on the loading route forever, or until the user refreshes (because it is in a bad state). If you add the error route, ember will then transition to the error route. Again, this is caused by findRecord erroring out. The adapter needs to hit line 346 for findRecord to complete successfully; it can never do that if pouchdb is too caught up in trying to load a large data set and the record you are looking for doesn't exist (yet - in the local data store).

Perhaps you reference queryRecord because it can return an empty data set without erroring out? I would agree that that is certainly a good step, but does not fix the problem; the user will still need to reload the application, or we have to load all of the changes all of the time (even for records that ember has not requested yet? if I recall correctly). In theory the best approach in my opinion would be to send the user to the live database while it is synchronizing to the local one. Once that is done, it never has to be done again as long as the user doesn't clear their cache.

The other side of it could be that @srsgores and myself are using this and it is an anti-pattern?

dweremeichik commented 8 years ago

For what it is worth, I did investigate using db.info() at one point to check the seq number after the database had been created in the "blueprint adapter". The problem is, db.info() is an asynchronous call, and Ember does not wait (as it shouldn't), so by the time you actually sort everything out and choose the correct database to load, it is already too late as the main adapter code /addon/adapters/pouch.js has already started to execute and will fail when it checks to make sure that the db property is set on the adapter.

This is what is leading me to believe that this needs to be fixed in the main adapter code. I'm more or less looking to see if anyone has any suggestions on if there is a better angle to approach this from.

nolanlawson commented 8 years ago

@dweremeichik It is impossible to have the seq ready synchronously when the script first runs. You have to go to IndexedDB first. Even if you were to try to cache it in LocalStorage (sync) it would just end up with race conditions between separate tabs/workers/etc.