Merge pull request #1706 from aminya/fix-updateMessages

lib/helpers.js

@@ -112,3 +112,92 @@ export function normalizeMessages(linterName: string, messages: Array<Message>)
     message.key = messageKey(message)
   }
 }
+
+// update the key of the given messages
+export function updateKeys(messages: Array<Message>) {
+  for (let i = 0, { length } = messages; i < length; ++i) {
+    messages[i].key = messageKey(messages[i])
+  }
+}
+
+// create a map from keys to messages
+export function createKeyMessageMap(messages: Array<Message>): Map<string, Message> {
+  const keyMessageMap = new Map()
+  for (let i = 0, { length } = messages; i < length; ++i) {
+    const message = messages[i]
+    keyMessageMap.set(message.key, message)
+  }
+  return keyMessageMap
+}
+
+interface FlaggedMessages {
+  oldKept: Array<Message>;
+  oldRemoved: Array<Message>;
+  newAdded: Array<Message>;
+}
+
+// This fast function returns the new messages and old messages by comparing their key against the cache.
+// This prevents re-rendering the already rendered messages
+export function flagMessages(inputs: Array<Message>, oldMessages: Array<Message>): FlaggedMessages | null {
+  // inputs check
+  if (inputs === undefined || oldMessages === undefined) {
+    return null
+  }
+
+  // All the messages of the linter are new, no need to diff
+  // tag the messages for adding and save them to linter's cache
+  if (!oldMessages.length) {
+    // NOTE: No need to add .key here because normalizeMessages already does that
+    return { oldKept: [], oldRemoved: [], newAdded: inputs }
+  }
+
+  // The linter has no messages anymore
+  // tag all of its messages from cache for removal and empty the cache
+  if (!inputs.length) {
+    return { oldKept: [], oldRemoved: oldMessages, newAdded: [] }
+  }
+
+  // In all the other situations:
+  // perform diff checking between the linter's new messages and its cache
+
+  // create a map from keys to oldMessages
+  const cache = createKeyMessageMap(oldMessages)
+
+  // Find old kept and new added
+  const newAdded: Set<Message> = new Set()
+  const oldKept: Map<string, Message> = new Map()
+  for (let iInput = 0, len = inputs.length; iInput < len; iInput++) {
+    const input = inputs[iInput]
+    if (cache.has(input.key)) {
+      oldKept.set(input.key, input)
+    } else {
+      newAdded.add(input)
+    }
+  }
+
+  // Find old removed
+  const cacheKeys = Array.from(cache.keys())
+  const oldKeptKeys = Array.from(oldKept.keys())
+
+  const oldRemovedKeys = cacheKeys.filter(x => !oldKeptKeys.includes(x))
+
+  const oldRemoved = new Set()
+  for (let iRemoved = 0, RemovedKeysLen = oldRemovedKeys.length; iRemoved < RemovedKeysLen; iRemoved++) {
+    oldRemoved.add(cache.get(oldRemovedKeys[iRemoved]))
+  }
+
+  return {
+    oldKept: Array.from(oldKept.values()),
+    oldRemoved: oldRemoved ? Array.from(oldRemoved) : [],
+    newAdded: Array.from(newAdded),
+  }
+}
+
+// fast mergeArray function
+// https://uilicious.com/blog/javascript-array-push-is-945x-faster-than-array-concat/
+export function mergeArray(arr1: Array<any>, arr2: Array<any>) {
+  if (!arr2.length) {
+    return
+  }
+  Array.prototype.push.apply(arr1, arr2)
+}

lib/message-registry.js

@@ -3,7 +3,7 @@
 import { CompositeDisposable, Emitter } from 'atom'
 import debounce from 'lodash/debounce'
 import type { Disposable, TextBuffer } from 'atom'
-import { messageKey } from './helpers'
+import { flagMessages, mergeArray } from './helpers'
 import type { MessagesPatch, Message, Linter } from './types'
 
 type Linter$Message$Map = {
@@ -32,6 +32,7 @@ class MessageRegistry {
     this.subscriptions.add(this.emitter)
   }
   set({ messages, linter, buffer }: { messages: Array<Message>, linter: Linter, buffer: TextBuffer }) {
+    // check if the linter has been already set
     let found = null
     for (const entry of this.messagesMap) {
       if (entry.buffer === buffer && entry.linter === linter) {
@@ -41,86 +42,52 @@ class MessageRegistry {
     }
 
     if (found) {
+      // found linter
       found.messages = messages
       found.changed = true
     } else {
+      // new linter
       this.messagesMap.add({ messages, linter, buffer, oldMessages: [], changed: true, deleted: false })
     }
     this.debouncedUpdate()
   }
   update() {
+    // the final object sent to UI that contains the messages tagged for adding/removeal. messages is all the kept + added messages
     const result = { added: [], removed: [], messages: [] }
 
+    // looping over each linter
     for (const entry of this.messagesMap) {
+      // if linter is deleted
+      // tag the linter's cache for removal and delete it from the map
       if (entry.deleted) {
-        result.removed = result.removed.concat(entry.oldMessages)
+        mergeArray(result.removed, entry.oldMessages)
         this.messagesMap.delete(entry)
         continue
       }
+
+      // if the linter is not changed
+      // just use its cache (no added/removed and everything is kept) and skip the rest
       if (!entry.changed) {
-        result.messages = result.messages.concat(entry.oldMessages)
-        continue
-      }
-      entry.changed = false
-      if (!entry.oldMessages.length) {
-        // All messages are new, no need to diff
-        // NOTE: No need to add .key here because normalizeMessages already does that
-        result.added = result.added.concat(entry.messages)
-        result.messages = result.messages.concat(entry.messages)
-        entry.oldMessages = entry.messages
+        // TODO When this code acutally runs?!
+        mergeArray(result.messages, entry.oldMessages)
         continue
       }
-      if (!entry.messages.length) {
-        // All messages are old, no need to diff
-        result.removed = result.removed.concat(entry.oldMessages)
-        entry.oldMessages = []
-        continue
-      }
-
-      const newKeys = new Set()
-      const oldKeys = new Set()
-      const { oldMessages } = entry
-      let foundNew = false
-      entry.oldMessages = []
-
-      for (let i = 0, { length } = oldMessages; i < length; ++i) {
-        const message = oldMessages[i]
-        message.key = messageKey(message)
-        oldKeys.add(message.key)
-      }
-
-      for (let i = 0, { length } = entry.messages; i < length; ++i) {
-        const message = entry.messages[i]
-        if (newKeys.has(message.key)) {
-          continue
-        }
-        newKeys.add(message.key)
-        if (!oldKeys.has(message.key)) {
-          foundNew = true
-          result.added.push(message)
-          result.messages.push(message)
-          entry.oldMessages.push(message)
-        }
-      }
 
-      if (!foundNew && entry.messages.length === oldMessages.length) {
-        // Messages are unchanged
-        result.messages = result.messages.concat(oldMessages)
-        entry.oldMessages = oldMessages
-        continue
-      }
+      // flag messages as oldKept, oldRemoved, newAdded
+      const flaggedMessages = flagMessages(entry.messages, entry.oldMessages)
 
-      for (let i = 0, { length } = oldMessages; i < length; ++i) {
-        const message = oldMessages[i]
-        if (newKeys.has(message.key)) {
-          entry.oldMessages.push(message)
-          result.messages.push(message)
-        } else {
-          result.removed.push(message)
-        }
+      // update the result and cache
+      if (flaggedMessages !== null) {
+        const { oldKept, oldRemoved, newAdded } = flaggedMessages
+        mergeArray(result.added, newAdded)
+        mergeArray(result.removed, oldRemoved)
+        const allThisEntry = newAdded.concat(oldKept)
+        mergeArray(result.messages, allThisEntry)
+        entry.oldMessages = allThisEntry // update chache
       }
     }
 
+    // if any messages is removed or added, then update the UI
     if (result.added.length || result.removed.length) {
       this.messages = result.messages
       this.emitter.emit('did-update-messages', result)

package.json

@@ -14,7 +14,8 @@
   "private": true,
   "scripts": {
     "test": "apm test",
-    "lint": "(flow check) && (eslint . ) && (prettier --list-different lib/*.js)"
+    "lint": "(flow check) && (eslint . ) && (prettier --list-different lib/*.js)",
+    "lint:fix": "eslint . --fix && prettier --write lib/**/*.js"
   },
   "engines": {
     "atom": ">=1.14.0 <2.0.0"

spec/common.js

@@ -18,13 +18,13 @@ export function getLinter(): Object {
     },
   }
 }
-export function getMessage(filePathOrNormalized: ?(boolean | string)): Object {
+export function getMessage(filePathOrNormalized: ?(boolean | string) = undefined): Object {
   const message: Object = {
     severity: 'error',
     excerpt: String(Math.random()),
     location: { file: __filename, position: [[0, 0], [0, 0]] },
   }
-  if (typeof filePathOrNormalized === 'boolean' && filePathOrNormalized) {
+  if ((typeof filePathOrNormalized === 'boolean' && filePathOrNormalized) || filePathOrNormalized === undefined) {
     normalizeMessages('Some Linter', [message])
   } else if (typeof filePathOrNormalized === 'string') {
     message.location.file = filePathOrNormalized