fix(api): Fix to remove instance (#357)

When .destroy() is called, it should be removed from the bb.instance Fix #347 Close #357
naver · Mar 29, 2018 · 80fdc31 · 80fdc31
1 parent fb12b03
commit 80fdc31
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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;