WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit ce86f35

Browse files
authored
Merge pull request #5761 from ag-grid/ag-16389/fix-mixed-update-updateDelta-applyTransaction
AG-16389 - Fix mixed data update types yielding corrupt update.
2 parents f818677 + 3697dc5 commit ce86f35

File tree

3 files changed

+82
-1
lines changed

3 files changed

+82
-1
lines changed

packages/ag-charts-community/src/chart/chart.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,77 @@ describe('Chart', () => {
500500

501501
expect(chart.data.data.length).toBe(lengthBeforeMutation);
502502
});
503+
504+
// AG-16389: updateDelta should not reset data accumulated via applyTransaction
505+
it('should preserve applyTransaction data when updateDelta changes series options', async () => {
506+
const initialData = [
507+
{ x: 0, y: 10 },
508+
{ x: 1, y: 20 },
509+
];
510+
511+
const options = prepareTestOptions<{
512+
data: { x: number; y: number }[];
513+
series: any[];
514+
}>({
515+
data: initialData,
516+
series: [
517+
{
518+
type: 'line',
519+
xKey: 'x',
520+
yKey: 'y',
521+
connectMissingData: false,
522+
},
523+
],
524+
});
525+
526+
// Step 1: Create chart with initial data
527+
const chartProxy = AgCharts.create(options);
528+
chart = deproxy(chartProxy);
529+
await waitForChartStability(chart);
530+
expect(chart.data.data.length).toBe(2);
531+
532+
// Step 2: updateDelta with increasing length data-set (simulates loading data)
533+
await chartProxy.updateDelta({
534+
data: [
535+
{ x: 0, y: 10 },
536+
{ x: 1, y: 20 },
537+
{ x: 2, y: 30 },
538+
],
539+
});
540+
await waitForChartStability(chart);
541+
expect(chart.data.data.length).toBe(3);
542+
543+
// Step 3: Full update back to initialData (simulates user action that resets data)
544+
await chartProxy.updateDelta({ data: initialData });
545+
await waitForChartStability(chart);
546+
expect(chart.data.data.length).toBe(2);
547+
548+
// At this point, DataSet.data and userOptions.data may have different references
549+
// Step 4: Use applyTransaction to add more data (streaming scenario)
550+
await chartProxy.applyTransaction({
551+
add: [
552+
{ x: 2, y: 30 },
553+
{ x: 3, y: 40 },
554+
],
555+
});
556+
await waitForChartStability(chart);
557+
expect(chart.data.data.length).toBe(4);
558+
559+
// Step 5: Toggle series option - this should NOT reset the data
560+
await chartProxy.updateDelta({
561+
series: options.series.map((s) => ({ ...s, connectMissingData: true })),
562+
});
563+
await waitForChartStability(chart);
564+
565+
// Data should still have 4 items (not reset to initial 2)
566+
expect(chart.data.data.length).toBe(4);
567+
expect(chart.data.data).toEqual([
568+
{ x: 0, y: 10 },
569+
{ x: 1, y: 20 },
570+
{ x: 2, y: 30 },
571+
{ x: 3, y: 40 },
572+
]);
573+
});
503574
});
504575

505576
describe('Chart data inherited by Series', () => {

packages/ag-charts-community/src/chart/chart.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1389,7 +1389,10 @@ export abstract class Chart extends Observable implements ModuleInstance, ChartS
13891389
forceNodeDataRefresh = true;
13901390
}
13911391

1392-
if (deltaOptions.data) {
1392+
// AG-16389: Only reset data if the user explicitly passed 'data' in their delta.
1393+
const { userDeltaKeys } = newChartOptions;
1394+
const userExplicitlyPassedData = userDeltaKeys === undefined || userDeltaKeys.has('data');
1395+
if (deltaOptions.data && userExplicitlyPassedData) {
13931396
// Always create a new DataSet for updateDelta to ensure cache invalidation.
13941397
// Only clone when we still hold the caller's array reference (updateDelta fast path).
13951398
const suppliedData = deltaOptions.data;

packages/ag-charts-community/src/module/optionsModule.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ export class ChartOptions<T extends AgChartOptions = AgChartOptions> {
143143
identifiers: Set<string>;
144144
indices: Set<number>;
145145
}; // AG-16360
146+
userDeltaKeys?: Set<string>; // AG-16389: Track keys the user passed in deltaOptions
146147

147148
private static readonly debug = Debug.create(true, 'opts');
148149

@@ -165,6 +166,12 @@ export class ChartOptions<T extends AgChartOptions = AgChartOptions> {
165166
baseChartOptions = currentUserOptions;
166167
this.specialOverrides = baseChartOptions.specialOverrides;
167168

169+
// AG-16389: Track the keys the user explicitly passed in their delta.
170+
// This must be done before the fallback jsonDiff below, so we capture user intent.
171+
if (deltaOptions) {
172+
this.userDeltaKeys = new Set(Object.keys(deltaOptions));
173+
}
174+
168175
// No diff case - null means diff was a no-op.
169176
deltaOptions ??= jsonDiff(
170177
baseChartOptions.userOptions as T,

0 commit comments

Comments
 (0)