Skip to content

Commit 220ed32

Browse files
committed
fix: use continue instead of return in clearStacks loop
In commit 1e296cc, clearStacks was refactored from forEach to for...of, but the inner `return` (which previously only skipped the current forEach iteration) now exits the entire function. This means if any parsed data point lacks _stacks data for the current dataset (e.g. when two points share x-coordinates and their _stacks object, or when new points are inserted), the remaining data points are skipped and their stale stack entries are never cleared. This causes incorrect stacking when datasets are reordered. Fixes #12154
1 parent a153556 commit 220ed32

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

src/core/core.datasetController.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ function clearStacks(meta, items) {
217217
for (const parsed of items) {
218218
const stacks = parsed._stacks;
219219
if (!stacks || stacks[axis] === undefined || stacks[axis][datasetIndex] === undefined) {
220-
return;
220+
continue;
221221
}
222222
delete stacks[axis][datasetIndex];
223223
if (stacks[axis]._visualValues !== undefined && stacks[axis]._visualValues[datasetIndex] !== undefined) {

test/specs/core.datasetController.tests.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,39 @@ describe('Chart.DatasetController', function() {
952952
expect(meta._parsed[0]._stacks).toEqual(jasmine.objectContaining({y: {0: 10, 1: 20, _top: 1, _bottom: null, _visualValues: {0: 10, 1: 20}}}));
953953
});
954954

955+
it('should clear stacks for all data points even when some lack stack data', function() {
956+
// Regression test for #12154: clearStacks used `return` instead of
957+
// `continue`, so if any parsed item lacked _stacks data the remaining
958+
// items were skipped, leaving stale stack entries.
959+
var chart = acquireChart({
960+
type: 'line',
961+
data: {
962+
datasets: [{
963+
data: [{x: 1, y: 10}, {x: 2, y: 20}, {x: 3, y: 30}],
964+
label: 'A'
965+
}, {
966+
data: [{x: 1, y: 11}, {x: 2, y: 21}, {x: 3, y: 31}],
967+
label: 'B'
968+
}]
969+
},
970+
options: {
971+
parsing: false,
972+
scales: {
973+
y: {stacked: 'single'}
974+
}
975+
}
976+
});
977+
978+
// Swap dataset order
979+
var datasets = chart.data.datasets;
980+
chart.data.datasets = [datasets[1], datasets[0]];
981+
982+
// This should not throw or leave stale stack data
983+
expect(function() {
984+
chart.update();
985+
}).not.toThrow();
986+
});
987+
955988
describe('resolveDataElementOptions', function() {
956989
it('should cache options when possible', function() {
957990
const chart = acquireChart({

0 commit comments

Comments
 (0)