[jsfm] enhance the destroy logic of document and element (#1529)

* Recursively destroy all nodes within the `nodeMap` while destroying a document. * Manually remove all props in `node.destroy()`, even if it may not exist.
apache · Sep 17, 2018 · d7c40d2 · d7c40d2
1 parent 91fd803
commit d7c40d2
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 13 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -125,14 +125,12 @@
     "no-unreachable": 2,
     "no-unsafe-finally": 2,
     "no-unused-vars": [2, { "vars": "all", "args": "none" }],
-    "no-useless-call": 2,
     "no-useless-computed-key": 2,
     "no-useless-constructor": 2,
     "no-useless-escape": 2,
     "no-whitespace-before-property": 2,
     "no-with": 2,
     "one-var": [2, { "initialized": "never" }],
-    // "operator-linebreak": [2, "after", { "overrides": { "?": "before", ":": "before" } }],
     "padded-blocks": [2, "never"],
     "quotes": [2, "single", {"avoidEscape": true, "allowTemplateLiterals": true}],
     "semi": [2, "never"],

diff --git a/runtime/bridge/CallbackManager.js b/runtime/bridge/CallbackManager.js
@@ -78,8 +78,9 @@ export default class CallbackManager {
     if (typeof callback === 'function') {
       try {
         return callback.call(null, data)
-      } catch (error) {
-        console.error(`[JS Framework] Failed to execute the callback function:\n + ${error.toString()}`)
+      }
+      catch (error) {
+        console.error(`[JS Framework] Failed to execute the callback function:\n ${error.toString()}`)
       }
     }
     return new Error(`invalid callback id "${callbackId}"`)

diff --git a/runtime/vdom/Document.js b/runtime/vdom/Document.js
@@ -169,19 +169,40 @@ export default class Document {
     if (domChanges) {
       updateElement(el, domChanges)
     }
-    const isBubble = this.getRef('_root').attr['bubble'] === 'true'
-    return el.fireEvent(type, event, isBubble, options)
+    let result
+    const $root = this.getRef('_root')
+    if ($root && $root.attr) {
+      const isBubble = $root.attr['bubble'] === 'true'
+      result = el.fireEvent(type, event, isBubble, options)
+    }
+    return result
   }
 
   /**
   * Destroy current document, and remove itself form docMap.
   */
   destroy () {
-    this.taskCenter.destroyCallback()
+    removeDoc(this.id)
+    delete this.id
+    delete this.URL
+    delete this.documentElement
+    delete this.ownerDocument
+
+    // remove listener and taskCenter
     delete this.listener
-    delete this.nodeMap
+    this.taskCenter.destroyCallback()
     delete this.taskCenter
-    removeDoc(this.id)
+
+    // remove nodeMap
+    for (const id in this.nodeMap) {
+      try {
+        if (typeof this.nodeMap[id] !== 'undefined') {
+          this.nodeMap[id].destroy()
+        }
+      }
+      catch (e) {}
+    }
+    delete this.nodeMap
   }
 }
 

diff --git a/runtime/vdom/Element.js b/runtime/vdom/Element.js
@@ -457,7 +457,8 @@ export default class Element extends Node {
         else {
           result = handler.call(this, event)
         }
-      } catch (error) {
+      }
+      catch (error) {
         console.error(`[JS Framework] Failed to invoke the event handler of "${type}" `
           + `on ${this.type} (${this.ref}):\n ${error.toString()}`)
       }

diff --git a/runtime/vdom/Node.js b/runtime/vdom/Node.js
@@ -38,10 +38,34 @@ export default class Node {
     const doc = getDoc(this.docId)
     if (doc) {
       delete this.docId
+      delete this.ownerDocument
       delete doc.nodeMap[this.nodeId]
     }
-    this.children.forEach(child => {
-      child.destroy()
-    })
+
+    // node props
+    delete this.nodeId
+    delete this.ref
+    delete this.parentNode
+    delete this.nextSibling
+    delete this.previousSibling
+
+    // element props
+    delete this.nodeType
+    delete this.type
+    delete this.attr
+    delete this.style
+    delete this.classStyle
+    delete this.event
+    delete this.depth
+
+    // child nodes
+    try {
+      this.children.forEach(child => {
+        child.destroy()
+      })
+    }
+    catch (e) {}
+    delete this.children
+    delete this.pureChildren
   }
 }