perf(accounts): Cache accountForId - need ordered map...

In general, we call the functions in AccountStore and CategoryStore / constantly / and inside of critical places like thread list render. Would be nice to create a safe and generic way of caching things and invalidating them when data changes.

spec/models/query-subscription-spec.coffee

@@ -217,7 +217,7 @@ describe "QuerySubscription", ->
         subscription = new QuerySubscription(DatabaseStore.findAll(Thread))
         subscription.update()
         advanceClock()
-        expect(subscription._fetchRange).toHaveBeenCalledWith(QueryRange.infinite(), {entireModels: true})
+        expect(subscription._fetchRange).toHaveBeenCalledWith(QueryRange.infinite(), {entireModels: true, version: 1})
 
       it "should fetch full full models only when the previous set is empty", ->
         subscription = new QuerySubscription(DatabaseStore.findAll(Thread))
@@ -225,7 +225,7 @@ describe "QuerySubscription", ->
         subscription._set.addModelsInRange([new Thread()], new QueryRange(start: 0, end: 1))
         subscription.update()
         advanceClock()
-        expect(subscription._fetchRange).toHaveBeenCalledWith(QueryRange.infinite(), {entireModels: false})
+        expect(subscription._fetchRange).toHaveBeenCalledWith(QueryRange.infinite(), {entireModels: false, version: 1})
 
     describe "when the query has a range", ->
       beforeEach ->
@@ -237,7 +237,7 @@ describe "QuerySubscription", ->
           subscription._set = null
           subscription.update()
           advanceClock()
-          expect(subscription._fetchRange).toHaveBeenCalledWith(@query.range(), {entireModels: true})
+          expect(subscription._fetchRange).toHaveBeenCalledWith(@query.range(), {entireModels: true, version: 1})
 
       describe "when we have a previous range", ->
         it "should call _fetchRange for the ranges representing the difference", ->
@@ -255,5 +255,5 @@ describe "QuerySubscription", ->
           subscription.update()
           advanceClock()
           expect(subscription._fetchRange.callCount).toBe(2)
-          expect(subscription._fetchRange.calls[0].args).toEqual([customRange1, {entireModels: true}])
-          expect(subscription._fetchRange.calls[1].args).toEqual([customRange2, {entireModels: true}])
+          expect(subscription._fetchRange.calls[0].args).toEqual([customRange1, {entireModels: true, version: 1}])
+          expect(subscription._fetchRange.calls[1].args).toEqual([customRange2, {entireModels: true, version: 1}])

src/flux/stores/account-store.coffee

@@ -27,16 +27,21 @@ class AccountStore
     @listenTo Actions.removeAccount, @_onRemoveAccount
     @listenTo Actions.updateAccount, @_onUpdateAccount
 
-    NylasEnv.config.observe saveTokensKey, (updatedTokens) =>
+    @_caches = {}
+
+    NylasEnv.config.onDidChange saveTokensKey, (change) =>
+      updatedTokens = change.newValue
       return if _.isEqual(updatedTokens, @_tokens)
       newAccountIds = _.keys(_.omit(updatedTokens, _.keys(@_tokens)))
       @_load()
       if newAccountIds.length > 0
         Actions.focusDefaultMailboxPerspectiveForAccounts([newAccountIds[0]])
+
     if NylasEnv.isComposerWindow()
-      NylasEnv.config.observe saveObjectsKey, => @_load()
+      NylasEnv.config.onDidChange saveObjectsKey, => @_load()
 
   _load: =>
+    @_caches = {}
     @_accounts = []
     for json in NylasEnv.config.get(saveObjectsKey) || []
       @_accounts.push((new Account).fromJSON(json))
@@ -56,6 +61,7 @@ class AccountStore
     account = @_accounts[idx]
     return if !account
     account = _.extend(account, updated)
+    @_caches = {}
     @_accounts[idx] = account
     NylasEnv.config.set(saveObjectsKey, @_accounts)
     @trigger()
@@ -65,6 +71,7 @@ class AccountStore
     return if idx is -1
 
     delete @_tokens[id]
+    @_caches = {}
     @_accounts.splice(idx, 1)
     @_save()
 
@@ -84,6 +91,7 @@ class AccountStore
     @_tokens[json.id] = json.auth_token
 
     account = (new Account).fromJSON(json)
+    @_caches = {}
     @_accounts.push(account)
     @_save()
 
@@ -92,6 +100,13 @@ class AccountStore
 
   # Exposed Data
 
+  # Private: Helper which caches the results of getter functions often needed
+  # in the middle of React render cycles. (Performance critical!)
+
+  _cachedGetter: (key, fn) ->
+    @_caches[key] ?= fn()
+    @_caches[key]
+
   # Public: Returns an {Array} of {Account} objects
   accounts: =>
     @_accounts
@@ -109,25 +124,26 @@ class AccountStore
 
   # Public: Returns the {Account} for the given email address, or null.
   accountForEmail: (email) =>
-    _.find @_accounts, (account) ->
-      return true if Utils.emailIsEquivalent(email, account.emailAddress)
-      for alias in account.aliases
-        return true if Utils.emailIsEquivalent(email, alias)
-      return false
+    @_cachedGetter "accountForEmail:#{email}", =>
+      _.find @_accounts, (account) ->
+        return true if Utils.emailIsEquivalent(email, account.emailAddress)
+        for alias in account.aliases
+          return true if Utils.emailIsEquivalent(email, alias)
+        return false
 
   # Public: Returns the {Account} for the given account id, or null.
   accountForId: (id) =>
-    _.findWhere(@_accounts, {id})
+    @_cachedGetter "accountForId:#{id}", => _.findWhere(@_accounts, {id})
 
   aliases: =>
-    aliases = []
-    for acc in @_accounts
-      aliases.push(acc.me())
-      for alias in acc.aliases
-        aliasContact = acc.meUsingAlias(alias)
-        aliasContact.isAlias = true
-        aliases.push(aliasContact)
-    return aliases
+      aliases = []
+      for acc in @_accounts
+        aliases.push(acc.me())
+        for alias in acc.aliases
+          aliasContact = acc.meUsingAlias(alias)
+          aliasContact.isAlias = true
+          aliases.push(aliasContact)
+      return aliases
 
   aliasesFor: (accountsOrIds) =>
     ids = accountsOrIds.map (accOrId) ->
@@ -136,7 +152,7 @@ class AccountStore
 
   # Public: Returns the currently active {Account}.
   current: =>
-    throw new Error("I can't haz the account")
+    throw new Error("AccountStore.current() has been deprecated.")
 
   # Private: This method is going away soon, do not rely on it.
   #
@@ -153,6 +169,8 @@ class AccountStore
     Thread = require '../models/thread'
     Label = require '../models/label'
 
+    @_caches = {}
+
     labels = {}
     threads = []
     messages = []

src/mailbox-perspective.coffee

@@ -100,12 +100,12 @@ class MailboxPerspective
   # Subclasses should call `super` if they override these methods
   canArchiveThreads: =>
     for aid in @accountIds
-      return false unless CategoryStore.getArchiveCategory(AccountStore.accountForId(aid))
+      return false unless CategoryStore.getArchiveCategory(aid)
     return true
 
   canTrashThreads: =>
     for aid in @accountIds
-      return false unless CategoryStore.getTrashCategory(AccountStore.accountForId(aid))
+      return false unless CategoryStore.getTrashCategory(aid)
     return true
 
   isInbox: =>