hoodiehq / discussion

General discussions and questions about Hoodie
7 stars 1 forks source link

(client) remove hoodie.ready.then(...) #114

Closed gr2m closed 7 years ago

gr2m commented 7 years ago

All of Hoodie Client’s APIs had to become async as part of dropping localStorage. We now use PouchDB for all our local data persistance needs, including config / session data, and all of PouchDB’s APIs are asynchronous.

This was quite a big change. We removed most of Hoodie Client’s sub modules synchronous APIs. This is what’s left:

Because of these APIs, we had to introduce hoodie.ready.then(callback) which made the very first usage of hoodie much harder than before. We know it was just a temporary measure and now is the time to figure out how to get rid of it again.

for hoodie.connectionStatus.ok we don’t need to do anything. It's simply is undefined until data was loaded from the store. If the app developer needs to make sure that happened they can still do hoodie.connectionStatus.ready.then(() => if (hoodie.connectionStatus.ok) {...})

for the hoodie.account API I would suggest we

  1. get rid of hoodie.account.id, hoodie.account.username, hoodie.account.isSignedIn() and hoodie.account.hasInvalidSession() altogether.
  2. merge hoodie.account.get() and hoodie.account.fetch() (same with hoodie.account.profile.{get,fetch})
  3. Remove hoodie.account.ready as it won’t be needed any longer

Turns out that not all apps have usernames. And using the username as a way to tell if a user is signed in or not is not a good idea. Instead, we should educate the developer how to make these checks themselves.

I would suggest to call the new method .get() but make it behave like .fetch(), meaning it’s asynchronous. And I would like to add a new option local: if set to true, it will load the data from the local store only. local defaults to true if the user is not signed or if only the id is requested as the id property may never change.

hoodie.account.get(options) would still accept a string or an array of strings as before to only get specific properties. Additionally, it now also accepts an object with {local, properties} properties or a second {local} option.

Examples

// loads from local store if not signed in, otherwise fetches
// it from /hoodie/account/api and updates the local cache
hoodie.account.get().then(showAllAccountProperties)

// never fetches from remote because the account id cannot change
hoodie.account.get('id').then(showAccountId)

// check if user is signed in using the local username
hoodie.account.get('username', {local: true}).then(function (username) {
  if (username) {
    // user is signed in
  }
})

// check if user’s session is invalid
hoodie.account.get('session.invalid', {local: true}).then(function (hasInvalidSession) {
  if (hasInvalidSession) {
    // ask user to sign in again
  }
})

// get several properties from local store only
hoodie.account.get('session.invalid', {local: true}).then(function (hasInvalidSession) {
  if (hasInvalidSession) {
    // ask user to sign in again
  }
})
janl commented 7 years ago

+1 this looks good and is well thought out.

local defaults to true if the user is not signed or if only the id is requested as the id property may never change

I could see these special cases causing confusion by developers, but I can’t foresee how, I’d say this is a build, wait, and adjust type of scenario: let’s get this going and see if it does cause any issues. If not: yay ;)

gr2m commented 7 years ago

I also have the feeling that this is not yet the final API for "Hoodie 1.0", but it feels good to see the API evolve. I’ve a PR ready at https://github.com/hoodiehq/hoodie-account-client/pull/141