From 80fdc31209bb83789a1ae1f343380720c18fcdcd Mon Sep 17 00:00:00 2001 From: Jae Sung Park Date: Thu, 29 Mar 2018 11:06:06 +0900 Subject: [PATCH] fix(api): Fix to remove instance (#357) When .destroy() is called, it should be removed from the bb.instance Fix #347 Close #357 --- demo/chart.js | 29 ++++++++++++++--------------- spec/api/api.chart-spec.js | 3 +++ src/api/api.chart.js | 27 ++++++++++----------------- 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/demo/chart.js b/demo/chart.js index 634381b1f..03b533f47 100644 --- a/demo/chart.js +++ b/demo/chart.js @@ -10,7 +10,6 @@ var billboardDemo = { this.$title = document.getElementById("title"); this.$codeArea = document.querySelector(".code"); this.$code = document.querySelector("code"); - this.chartInst = []; this.WIDTH = 768; this.selectedClass = "selected"; @@ -114,25 +113,25 @@ var billboardDemo = { * @returns {boolean} */ generate: function(type, key) { - var chartInst = this.chartInst; + var inst = bb.instance; var typeData = demos[type][key]; var isArray = Array.isArray(typeData); var self = this; - chartInst.length && chartInst.forEach(function (c, i, array) { - c.timer && c.timer.forEach(function (v) { - clearTimeout(v); - }); - - if (c.element.parentNode) - c.element.parentNode.removeChild(c.element); - - //c.destroy(); - array.shift(); + inst.length && inst.forEach(function (c) { + var timer = c.timer; + var el = c.element; + + try { + timer && timer.forEach(function (v) { + clearTimeout(v); + }); + } finally { + el.parentNode && el.parentNode.removeChild(el); + c.destroy(); + } }); - this.chartInst = []; - var code = { markup: [], data: [] @@ -191,7 +190,6 @@ var billboardDemo = { var inst = bb.generate(options); inst.timer = []; - this.chartInst.push(inst); var codeStr = "var chart = bb.generate(" + JSON.stringify(options, function (k, v) { @@ -230,6 +228,7 @@ var billboardDemo = { if (index && index > 1) { code.data.push("\r\n\r\n"); } + // script this.$code.innerHTML code.data.push("// Script\r\n" + codeStr.replace(/"(\[|{)/, "$1").replace(/(\]|})"/, "$1")); diff --git a/spec/api/api.chart-spec.js b/spec/api/api.chart-spec.js index b08616e98..486378803 100644 --- a/spec/api/api.chart-spec.js +++ b/spec/api/api.chart-spec.js @@ -5,6 +5,7 @@ /* eslint-disable */ import util from "../assets/util"; import CLASS from "../../src/config/classes"; +import bb from "../../src/core"; describe("API chart", () => { let chart; @@ -111,6 +112,8 @@ describe("API chart", () => { chart.destroy(); expect(d3.select("#chart svg").empty()).to.be.true; + expect(Object.keys(chart).length).to.be.equal(0); + expect(bb.instance.indexOf(chart) === -1).to.be.true; }); }); }); diff --git a/src/api/api.chart.js b/src/api/api.chart.js index 1df27b18c..2a08bd981 100644 --- a/src/api/api.chart.js +++ b/src/api/api.chart.js @@ -56,30 +56,23 @@ extend(Chart.prototype, { destroy() { const $$ = this.internal; + $$.charts.splice($$.charts.indexOf(this), 1); window.clearInterval($$.intervalForObserveInserted); - if ($$.resizeTimeout !== undefined) { + $$.resizeTimeout !== undefined && window.clearTimeout($$.resizeTimeout); - } removeEvent(window, "resize", $$.resizeFunction); - // if (window.detachEvent) { - // window.detachEvent('onresize', $$.resizeFunction); - // } else if (window.removeEventListener) { - // window.removeEventListener('resize', $$.resizeFunction); - // } else { - // var wrapper = window.onresize; - // // check if no one else removed our wrapper and remove our resizeFunction from it - // if (wrapper && wrapper.add && wrapper.remove) { - // wrapper.remove($$.resizeFunction); - // } - // } - $$.selectChart.classed("bb", false).html(""); - // MEMO: this is needed because the reference of some elements will not be released, then memory leak will happen. - Object.keys($$).forEach(key => { - $$[key] = null; + // Reference of some elements will not be released, then memory leak will happen. + Object.keys(this).forEach(key => { + key === "internal" && Object.keys($$).forEach(k => { + $$[k] = null; + }); + + this[key] = null; + delete this[key]; }); return null;