diff --git a/projects/angular-highcharts/src/lib/chart.spec.ts b/projects/angular-highcharts/src/lib/chart.spec.ts index 9651f87..1c7dcda 100644 --- a/projects/angular-highcharts/src/lib/chart.spec.ts +++ b/projects/angular-highcharts/src/lib/chart.spec.ts @@ -1,3 +1,5 @@ +import { ElementRef } from '@angular/core'; +import Highcharts from 'highcharts/esm/highcharts.src'; import { Chart } from './chart'; /** @@ -159,4 +161,52 @@ describe('Chart', () => { expect(chart.ref).toBeUndefined(); }); }); + + describe('doubled export callback (#238)', () => { + // Regression guard for the `forExport`/broken-navigation crashes reported in + // #384, #327, #321, #316, #289 and #324. Highcharts' (offline) exporting + // renders a *temporary* copy of the chart via + // `new chart.constructor(options, chart.callback)` — reusing the very + // callback `init()` registered — and then destroys that copy itself. Without + // the `if (!this.ref)` guard the copy's callback overwrites `ref` with a + // chart Highcharts immediately frees, so the next `ref` access or the + // component's `destroy()` hits `Cannot read properties of undefined + // (reading 'forExport')`. + it('keeps ref pinned to the live chart when the export copy re-invokes the callback', () => { + const chart = new Chart(); + const live = makeFakeChart(); + const exportCopy = makeFakeChart(); + exportCopy.userOptions.title.text = 'export-copy'; + + // Stand in for Highcharts.chart(): capture the callback and fire it with + // the live chart, exactly as chart creation does. + let registered: ((c: unknown) => void) | undefined; + const spy = vi + .spyOn(Highcharts, 'chart') + .mockImplementation(((_el: unknown, _opts: unknown, cb: (c: unknown) => void) => { + registered = cb; + cb(live); + return live; + }) as never); + + chart.init({ nativeElement: document.createElement('div') } as ElementRef); + + // Now simulate the export: Highcharts re-invokes our callback with the + // throwaway copy, then frees it. + registered!(exportCopy); + exportCopy.destroy(); + + // ref stayed on the live chart; ref$ emitted it exactly once. + expect(chart.ref).toBe(live); + const emissions: unknown[] = []; + chart.ref$.subscribe(c => emissions.push(c)); + expect(emissions).toEqual([live]); + + // Component teardown tears down the live chart, never the freed copy. + chart.destroy(); + expect(live.destroy).toHaveBeenCalledTimes(1); + + spy.mockRestore(); + }); + }); }); diff --git a/projects/angular-highcharts/src/lib/highcharts-gantt.spec.ts b/projects/angular-highcharts/src/lib/highcharts-gantt.spec.ts index 75a4167..4041a61 100644 --- a/projects/angular-highcharts/src/lib/highcharts-gantt.spec.ts +++ b/projects/angular-highcharts/src/lib/highcharts-gantt.spec.ts @@ -1,3 +1,5 @@ +import { ElementRef } from '@angular/core'; +import Highcharts from 'highcharts/esm/highcharts-gantt.src'; import { HighchartsGantt } from './highcharts-gantt'; function makeFakeChart() { @@ -31,4 +33,34 @@ describe('HighchartsGantt', () => { const chart = new HighchartsGantt(); expect(() => chart.destroy()).not.toThrow(); }); + + // Regression guard for the doubled export callback (#238): Highcharts renders + // a throwaway copy of the chart for export via + // `new chart.constructor(options, chart.callback)` and then frees it. The + // `if (!this.ref)` guard must ignore that second callback so `ref` never ends + // up pointing at a destroyed chart (the `forExport` crash). + it('keeps ref pinned to the live chart when the export copy re-invokes the callback', () => { + const chart = new HighchartsGantt(); + const live = makeFakeChart(); + const exportCopy = makeFakeChart(); + + let registered: ((c: unknown) => void) | undefined; + const spy = vi + .spyOn(Highcharts, 'ganttChart') + .mockImplementation(((_el: unknown, _opts: unknown, cb: (c: unknown) => void) => { + registered = cb; + cb(live); + return live; + }) as never); + + chart.init({ nativeElement: document.createElement('div') } as ElementRef); + registered!(exportCopy); + exportCopy.destroy(); + + expect(chart.ref).toBe(live); + chart.destroy(); + expect(live.destroy).toHaveBeenCalledTimes(1); + + spy.mockRestore(); + }); }); diff --git a/projects/angular-highcharts/src/lib/mapchart.spec.ts b/projects/angular-highcharts/src/lib/mapchart.spec.ts index 1a3e52f..f08e9e3 100644 --- a/projects/angular-highcharts/src/lib/mapchart.spec.ts +++ b/projects/angular-highcharts/src/lib/mapchart.spec.ts @@ -1,3 +1,5 @@ +import { ElementRef } from '@angular/core'; +import Highmaps from 'highcharts/esm/highmaps.src'; import { MapChart } from './mapchart'; function makeFakeChart() { @@ -31,4 +33,34 @@ describe('MapChart', () => { const chart = new MapChart(); expect(() => chart.destroy()).not.toThrow(); }); + + // Regression guard for the doubled export callback (#238): Highcharts renders + // a throwaway copy of the chart for export via + // `new chart.constructor(options, chart.callback)` and then frees it. The + // `if (!this.ref)` guard must ignore that second callback so `ref` never ends + // up pointing at a destroyed chart (the `forExport` crash). + it('keeps ref pinned to the live chart when the export copy re-invokes the callback', () => { + const chart = new MapChart(); + const live = makeFakeChart(); + const exportCopy = makeFakeChart(); + + let registered: ((c: unknown) => void) | undefined; + const spy = vi + .spyOn(Highmaps, 'mapChart') + .mockImplementation(((_el: unknown, _opts: unknown, cb: (c: unknown) => void) => { + registered = cb; + cb(live); + return live; + }) as never); + + chart.init({ nativeElement: document.createElement('div') } as ElementRef); + registered!(exportCopy); + exportCopy.destroy(); + + expect(chart.ref).toBe(live); + chart.destroy(); + expect(live.destroy).toHaveBeenCalledTimes(1); + + spy.mockRestore(); + }); }); diff --git a/projects/angular-highcharts/src/lib/stockchart.spec.ts b/projects/angular-highcharts/src/lib/stockchart.spec.ts index 2f20dcf..43c389d 100644 --- a/projects/angular-highcharts/src/lib/stockchart.spec.ts +++ b/projects/angular-highcharts/src/lib/stockchart.spec.ts @@ -1,3 +1,5 @@ +import { ElementRef } from '@angular/core'; +import Highstock from 'highcharts/esm/highstock.src'; import { StockChart } from './stockchart'; function makeFakeChart() { @@ -31,4 +33,34 @@ describe('StockChart', () => { const chart = new StockChart(); expect(() => chart.destroy()).not.toThrow(); }); + + // Regression guard for the doubled export callback (#238): Highcharts renders + // a throwaway copy of the chart for export via + // `new chart.constructor(options, chart.callback)` and then frees it. The + // `if (!this.ref)` guard must ignore that second callback so `ref` never ends + // up pointing at a destroyed chart (the `forExport` crash). + it('keeps ref pinned to the live chart when the export copy re-invokes the callback', () => { + const chart = new StockChart(); + const live = makeFakeChart(); + const exportCopy = makeFakeChart(); + + let registered: ((c: unknown) => void) | undefined; + const spy = vi + .spyOn(Highstock, 'stockChart') + .mockImplementation(((_el: unknown, _opts: unknown, cb: (c: unknown) => void) => { + registered = cb; + cb(live); + return live; + }) as never); + + chart.init({ nativeElement: document.createElement('div') } as ElementRef); + registered!(exportCopy); + exportCopy.destroy(); + + expect(chart.ref).toBe(live); + chart.destroy(); + expect(live.destroy).toHaveBeenCalledTimes(1); + + spy.mockRestore(); + }); });