fix(performance): Thread-list re-renders too often

Summary:
Freeze threads on their way out of the ThreadStore

Don't add an unread attribute to contacts by accident...

Don't pass inline functions as props that are looked at by _isEqual

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://review.inboxapp.com/D1367

internal_packages/thread-list/lib/thread-list-participants.cjsx

@@ -16,20 +16,20 @@ ThreadListParticipants = React.createClass
     if @props.thread.messageMetadata and @props.thread.messageMetadata.length > 1
       count = " (#{@props.thread.messageMetadata.length})"
 
-    chips = items.map (item, idx) ->
+    chips = items.map ({spacer, contact, unread}, idx) ->
-      if item.spacer
+      if spacer
         <span key={idx}>...</span>
       else
-        if item.name.length > 0
+        if contact.name.length > 0
           if items.length > 1
-            short = item.displayFirstName()
+            short = contact.displayFirstName()
           else
-            short = item.displayName()
+            short = contact.displayName()
         else
-          short = item.email
+          short = contact.email
         if idx < items.length-1 and not items[idx+1].spacer
           short += ", "
-        <span key={idx} className="unread-#{item.unread}">{short}</span>
+        <span key={idx} className="unread-#{unread}">{short}</span>
 
     <div className="participants">
       {chips}{count}
@@ -44,10 +44,11 @@ ThreadListParticipants = React.createClass
       last = null
       for msg in @props.thread.messageMetadata
         from = msg.from[0]
-        continue unless from
+        if from and from.email isnt last
-        if from.email isnt last
+          list.push({
-          from.unread = msg.unread
+            contact: msg.from[0]
-          list.push(from)
+            unread: msg.unread
+          })
           last = from.email
 
     else

internal_packages/thread-list/lib/thread-list.cjsx

@@ -18,6 +18,7 @@ ThreadList = React.createClass
     @_getStateFromStores()
  
   componentDidMount: ->
+    @_prepareColumns()
     @thread_store_unsubscribe = ThreadStore.listen @_onChange
     @thread_unsubscriber = atom.commands.add '.thread-list', {
       'thread-list:star-thread': => @_onStarThread()
@@ -40,18 +41,29 @@ ThreadList = React.createClass
     @body_unsubscriber.dispose()
 
   render: ->
+    # IMPORTANT: DO NOT pass inline functions as props. _.isEqual thinks these
+    # are "different", and will re-render everything. Instead, declare them with ?=,
+    # pass a reference. (Alternatively, ignore these in children's shouldComponentUpdate.)
+    #
+    # BAD:   onSelect={ (item) -> Actions.selectThreadId(item.id) }
+    # GOOD:  onSelect={@_onSelectItem}
+    #
     classes = React.addons.classSet("thread-list": true, "ready": @state.ready)
+
+    @_itemClassProvider ?= (item) -> if item.isUnread() then 'unread' else ''
+    @_itemOnSelect ?= (item) -> Actions.selectThreadId(item.id)
+
     <div className={classes}>
       <ListTabular
-        columns={@state.columns}
+        columns={@_columns}
         items={@state.items}
-        itemClassProvider={ (item) -> if item.isUnread() then 'unread' else '' }
+        itemClassProvider={@_itemClassProvider}
         selectedId={@state.selectedId}
-        onSelect={ (item) -> Actions.selectThreadId(item.id) } />
+        onSelect={@_itemOnSelect} />
       <Spinner visible={!@state.ready} />
     </div>
 
-  _computeColumns: ->
+  _prepareColumns: ->
     myEmail = NamespaceStore.current()?.emailAddress
 
     labelComponents = (thread) =>
@@ -101,7 +113,7 @@ ThreadList = React.createClass
       resolver: (thread) ->
         <span className="timestamp">{timestamp(thread.lastMessageTimestamp)}</span>
 
-    [c1, c2, c3, c4]
+    @_columns = [c1, c2, c3, c4]
 
   _onFocusSelectedIndex: ->
     Actions.selectThreadId(@state.selectedId)
@@ -145,5 +157,4 @@ ThreadList = React.createClass
   _getStateFromStores: ->
     ready: not ThreadStore.itemsLoading()
     items: ThreadStore.items()
-    columns: @_computeColumns()
     selectedId: ThreadStore.selectedId()

src/flux/models/thread.coffee

@@ -41,12 +41,12 @@ class Thread extends Model
   @naturalSortOrder: ->
     Thread.attributes.lastMessageTimestamp.descending()
 
-  fromJSON: (json) =>
+  fromJSON: (json) ->
     super(json)
     @unread = @isUnread()
     @
 
-  tagIds: =>
+  tagIds: ->
     _.map @tags, (tag) -> tag.id
   
   hasTagId: (id) ->

src/flux/models/utils.coffee

@@ -62,6 +62,13 @@ Utils =
     object.fromJSON(json)
     object
 
+  modelFreeze: (o) ->
+    Object.freeze(o)
+    for key, prop of o
+      if !o.hasOwnProperty(key) || typeof prop isnt 'object' || Object.isFrozen(prop)
+        continue
+      Utils.modelFreeze(prop)
+
   modelReviver: (k, v) ->
     return v if k == ""
     v = Utils.modelFromJSON(v) if (v instanceof Object && v['object'])

src/flux/stores/thread-store.coffee

@@ -5,6 +5,7 @@ WorkspaceStore = require './workspace-store'
 AddRemoveTagsTask = require '../tasks/add-remove-tags'
 MarkThreadReadTask = require '../tasks/mark-thread-read'
 Actions = require '../actions'
+Utils = require '../models/utils'
 Thread = require '../models/thread'
 Message = require '../models/message'
 _ = require 'underscore-plus'
@@ -60,6 +61,11 @@ ThreadStore = Reflux.createStore
         for item in items
           item.messageMetadata = results[item.id]
 
+          # Prevent anything from mutating these objects or their nested objects.
+          # Accidentally modifying items somewhere downstream (in a component)
+          # can trigger awful re-renders
+          Utils.modelFreeze(item)
+
         @_items = items
 
         # Sometimes we can ask for a thread that's not in the current set
@@ -90,6 +96,7 @@ ThreadStore = Reflux.createStore
 
         @_autoselectForLayoutMode()
 
+
         console.log("Fetching data for thread list took #{Date.now() - start} msec")
 
         # If we've loaded threads, remove the loading indicator.