Skip to content

Commit

Permalink
fix(api): Remove cached data when .unload() is called
Browse files Browse the repository at this point in the history
- Remove unnecessary .hasCaches()
- Add .removeCache()
- Enforce .unload API doc
- Remove cached data when is unloaded

Fix #626
Close #627
  • Loading branch information
netil committed Oct 25, 2018
1 parent 1e465ae commit aa91bab
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 26 deletions.
14 changes: 13 additions & 1 deletion spec/api/api.load-spec.js
Expand Up @@ -275,7 +275,19 @@ describe("API load", function() {
}, 500);
}
});
});

it("check for .unload()", () => {
const target = "data2";

})
// when
chart.unload({
ids: target,
done: () => {
expect(chart.data(target).length).to.be.equal(0);
expect(chart.internal.getCache(target)).to.be.null;
}
});
});
});
});
22 changes: 12 additions & 10 deletions src/api/api.load.js
Expand Up @@ -71,12 +71,6 @@ extend(Chart.prototype, {
config.data_colors[id] = args.colors[id];
});

// use cache if exists
if ("cacheIds" in args && $$.hasCaches(args.cacheIds, true)) {
$$.load($$.getCache(args.cacheIds, true), args.done);
return;
}

// unload if needed
if ("unload" in args && args.unload !== false) {
// TODO: do not unload if target will load (included in url/rows/columns)
Expand All @@ -98,12 +92,17 @@ extend(Chart.prototype, {
* @instance
* @memberOf Chart
* @param {Object} args
* - If ids given, the data that has specified target id will be unloaded. ids should be String or Array. If ids is not specified, all data will be unloaded.
* - If done given, the specified function will be called after data loded.
* | key | Type | Description |
* | --- | --- | --- |
* | ids | String | Array | Target id data to be unloaded. If not given, all data will be unloaded. |
* | done | Fuction | Callback after data is unloaded. |
* @example
* // Unload data2 and data3
* chart.unload({
* ids: ["data2", "data3"]
* ids: ["data2", "data3"],
* done: function() {
* // called after the unloaded
* }
* });
*/
unload(argsValue) {
Expand All @@ -116,13 +115,16 @@ extend(Chart.prototype, {
args = {ids: [args]};
}

$$.unload($$.mapToTargetIds(args.ids), () => {
const ids = $$.mapToTargetIds(args.ids);

$$.unload(ids, () => {
$$.redraw({
withUpdateOrgXDomain: true,
withUpdateXDomain: true,
withLegend: true
});

$$.removeCache(ids);
args.done && args.done();
});
}
Expand Down
39 changes: 24 additions & 15 deletions src/internals/cache.js
Expand Up @@ -3,27 +3,36 @@
* billboard.js project is licensed under the MIT license
*/
import ChartInternal from "./ChartInternal";
import {extend} from "./util";
import {toArray, extend} from "./util";

extend(ChartInternal.prototype, {
hasCaches(key, isDataType = false) {
if (isDataType) {
for (let i = 0, len = key.length; i < len; i++) {
if (!(key[i] in this.cache)) {
return false;
}
}

return true;
} else {
return key in this.cache;
}
},

/**
* Add cache
* @param {String} key
* @param {*} value
* @param {Boolean} isDataType
* @private
*/
addCache(key, value, isDataType = false) {
this.cache[key] = isDataType ? this.cloneTarget(value) : value;
},

/**
* Remove cache
* @param {String|Array} key
* @private
*/
removeCache(key) {
toArray(key).forEach(v => delete this.cache[v]);
},

/**
* Get cahce
* @param {String|Array} key
* @param {Boolean} isDataType
* @return {*}
* @private
*/
getCache(key, isDataType = false) {
if (isDataType) {
const targets = [];
Expand Down

0 comments on commit aa91bab

Please sign in to comment.