bugfix/437-performance-regression-with-many-charts

README.md

@@ -103,9 +103,6 @@ export const appConfig: ApplicationConfig = {
           import('highcharts/esm/themes/sunset'),
         ];
       },
-
-      // Optional: override the default 500ms delay before chart creation
-      timeout: 0,
     }),
     // Other providers here ...
   ],
@@ -358,7 +355,10 @@ import { HighchartsChartDirective } from 'highcharts-angular';
           import('highcharts/esm/modules/exporting'),
         ];
       },
-      timeout: 900, // Optional: increase timeout for loading modules
+      // Optional escape hatch: extra delay (ms) before the chart is created,
+      // after Highcharts and modules are ready (e.g. to let the container settle).
+      // Rarely needed — module loading is awaited automatically. Defaults to 0.
+      timeout: 0,
     }),
   ],
 })
@@ -443,8 +443,7 @@ export class StockComponent {
 **Note:**
 
 - Some Highcharts modules have dependencies and must be loaded in a specific order.
-- In such cases, use a promise chain (e.g., `import('highcharts/esm/highcharts-more').then(() => import('highcharts/esm/modules/dumbbell'))`)
-- instead of just listing them as array items. This ensures the dependent module loads only after its dependency.
+- Use a promise chain instead of listing dependent modules as separate array items, for example `import('highcharts/esm/highcharts-more').then(() => import('highcharts/esm/modules/dumbbell'))`. For `tilemap`, load `map` first: `import('highcharts/esm/modules/map').then(() => import('highcharts/esm/modules/tilemap'))`.
 
 ### To load a wrapper
 

highcharts-angular/src/lib/highcharts-chart.component.spec.ts

@@ -52,7 +52,6 @@ describe('TestComponent / HighchartsChartService (load module)', () => {
     title: string;
     modules: ModuleFactoryFunction;
     assert(hc: any): void;
-    timeout?: number;
   };
 
   const CASES: Case[] = [
@@ -78,7 +77,6 @@ describe('TestComponent / HighchartsChartService (load module)', () => {
         expect(hc.seriesTypes?.arearange).toBeDefined();
         expect(hc.seriesTypes?.dumbbell).toBeDefined();
       },
-      timeout: 2000,
     },
     {
       title: 'pattern-fill → adds SVGRenderer.addPattern',
@@ -104,7 +102,7 @@ describe('TestComponent / HighchartsChartService (load module)', () => {
       // 3) Provide the module(s) at the component level. This mirrors your real component’s
       //    `providers: [providePartialHighcharts({ modules: () => [...] })]`.
       TestBed.overrideComponent(TestComponent, {
-        set: { providers: [providePartialHighcharts({ modules: c.modules, timeout: c.timeout })] },
+        set: { providers: [providePartialHighcharts({ modules: c.modules })] },
       });
 
       const fixture = TestBed.createComponent(TestComponent);
@@ -134,7 +132,6 @@ describe('TestComponent / HighchartsChartService (load module)', () => {
         providers: [
           providePartialHighcharts({
             modules: () => [
-              import('highcharts/esm/modules/map'),
               import('highcharts/esm/modules/map').then(() => import('highcharts/esm/modules/tilemap')),
               import('highcharts/esm/modules/gantt'),
               import('highcharts/esm/highcharts-more').then(() => import('highcharts/esm/modules/dumbbell')),

highcharts-angular/src/lib/highcharts-chart.directive.spec.ts

@@ -1,4 +1,4 @@
-import { TestBed } from '@angular/core/testing';
+import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { ChangeDetectionStrategy, Component, DebugElement } from '@angular/core';
 import { By } from '@angular/platform-browser';
 import { HighchartsChartDirective } from './highcharts-chart.directive';
@@ -18,12 +18,27 @@ class TestHostComponent {
 }
 
 describe('HighchartsChartDirective', () => {
+  let fixture: ComponentFixture<TestHostComponent>;
   let debugElement: DebugElement;
   let directive: HighchartsChartDirective;
   let loadSpy: Spy;
+  let chartFactorySpy: Spy;
 
   beforeEach(() => {
-    loadSpy = jasmine.createSpy('load');
+    chartFactorySpy = jasmine.createSpy('chart').and.returnValue({
+      destroy: jasmine.createSpy('destroy'),
+      renderer: {
+        forExport: false,
+      },
+      update: jasmine.createSpy('update'),
+    } as unknown as Partial<Highcharts.Chart>);
+
+    const mockHighcharts = {
+      chart: chartFactorySpy,
+    } as unknown as typeof Highcharts;
+
+    loadSpy = jasmine.createSpy('load').and.resolveTo(mockHighcharts);
+
     TestBed.configureTestingModule({
       imports: [TestHostComponent, HighchartsChartDirective],
       providers: [
@@ -35,13 +50,12 @@ describe('HighchartsChartDirective', () => {
           provide: HighchartsChartService,
           useValue: {
             load: loadSpy,
-            highcharts: () => null,
           },
         },
       ],
     });
 
-    const fixture = TestBed.createComponent(TestHostComponent);
+    fixture = TestBed.createComponent(TestHostComponent);
     fixture.detectChanges();
     debugElement = fixture.debugElement.query(By.directive(HighchartsChartDirective));
     directive = debugElement.injector.get(HighchartsChartDirective);
@@ -58,4 +72,13 @@ describe('HighchartsChartDirective', () => {
   it('should load global config on initialization', () => {
     expect(loadSpy).toHaveBeenCalledWith({});
   });
+
+  it('should create the chart as soon as Highcharts is loaded', async () => {
+    expect(chartFactorySpy).not.toHaveBeenCalled();
+
+    await fixture.whenStable();
+    fixture.detectChanges();
+
+    expect(chartFactorySpy).toHaveBeenCalled();
+  });
 });

highcharts-angular/src/lib/highcharts-chart.directive.ts

@@ -1,5 +1,4 @@
 import {
-  computed,
   DestroyRef,
   Directive,
   effect,
@@ -8,13 +7,15 @@ import {
   input,
   model,
   output,
-  untracked,
+  PendingTasks,
   PLATFORM_ID,
+  signal,
+  untracked,
 } from '@angular/core';
 import { isPlatformServer } from '@angular/common';
 import { HighchartsChartService } from './highcharts-chart.service';
 import { HIGHCHARTS_CONFIG, HIGHCHARTS_TIMEOUT } from './highcharts-chart.token';
-import { ChartConstructorType, ConstructorChart } from './types';
+import { ChartConstructorType, ConstructorChart, HighchartsWithModuleConstructors } from './types';
 import type Highcharts from 'highcharts/esm/highcharts';
 
 @Directive({
@@ -23,6 +24,7 @@ import type Highcharts from 'highcharts/esm/highcharts';
 export class HighchartsChartDirective {
   /**
    * Type of the chart constructor.
+   * Changing it recreates the chart because Highcharts constructors are not update-compatible.
    */
   public readonly constructorType = input<ChartConstructorType>('chart');
 
@@ -47,94 +49,138 @@ export class HighchartsChartDirective {
    */
   public readonly update = model<boolean>(true);
 
-  public readonly chartInstance = output<Highcharts.Chart>(); // #26
+  public readonly chartInstance = output<Highcharts.Chart>();
 
   private readonly destroyRef = inject(DestroyRef);
-
   private readonly el = inject<ElementRef<HTMLElement>>(ElementRef);
-
   private readonly platformId = inject(PLATFORM_ID);
-
-  private readonly relativeConfig = inject(HIGHCHARTS_CONFIG, {
-    optional: true,
-  });
-
-  private readonly timeout = inject(HIGHCHARTS_TIMEOUT, {
-    optional: true,
-  });
-
+  private readonly relativeConfig = inject(HIGHCHARTS_CONFIG, { optional: true });
+  private readonly timeout = inject(HIGHCHARTS_TIMEOUT, { optional: true });
   private readonly highchartsChartService = inject(HighchartsChartService);
+  private readonly pendingTasks = inject(PendingTasks);
 
-  private chartCreated = false;
-
-  private _chartInstance: Highcharts.Chart | undefined;
-
+  private readonly loadedHighcharts = signal<typeof Highcharts | null>(null);
+  private readonly chart = signal<Highcharts.Chart | null>(null);
   private isDestroyed = false;
 
   private delay(ms: number): Promise<void> {
     return new Promise(resolve => setTimeout(resolve, ms));
   }
 
-  // Create the chart as soon as we can
-  private readonly chart = computed(async () => {
-    const highCharts = this.highchartsChartService.highcharts();
-    const constructorType = this.constructorType();
-    await this.delay(this.relativeConfig?.timeout ?? this.timeout ?? 500);
-    if (!highCharts) return;
-    const callback: Highcharts.ChartCallbackFunction = (chart: Highcharts.Chart) => {
-      if (chart.renderer.forExport || this.isDestroyed) return;
-      return this.chartInstance.emit(chart);
-    };
-    const chartFactories: Record<ChartConstructorType, ConstructorChart> = {
-      chart: highCharts.chart,
-      ganttChart: (highCharts as any).ganttChart,
-      mapChart: (highCharts as any).mapChart,
-      stockChart: (highCharts as any).stockChart,
-    };
-    return chartFactories[constructorType](
-      this.el.nativeElement,
-      // Use untracked, so we don't re-create new chart everytime options change
-      untracked(() => this.options()),
-      // Use Highcharts callback to emit chart instance, so it is available as early
-      // as possible. So that Angular is already aware of the instance if Highcharts raise
-      // events during the initialization that happens before coming back to Angular
-      callback,
-    );
-  });
+  private getChartFactory(highcharts: typeof Highcharts, constructorType: ChartConstructorType): ConstructorChart {
+    if (constructorType === 'chart') {
+      return highcharts.chart;
+    }
+
+    const highchartsWithModuleConstructors: HighchartsWithModuleConstructors = highcharts;
+    const chartFactory = highchartsWithModuleConstructors[constructorType];
+
+    if (!chartFactory) {
+      throw new Error(
+        `Highcharts constructor "${constructorType}" is not available. Did you load the required module?`,
+      );
+    }
+
+    return chartFactory;
+  }
+
+  private createChart(): void {
+    effect(onCleanup => {
+      const highcharts = this.loadedHighcharts();
+      const constructorType = this.constructorType();
+
+      if (!highcharts || this.isDestroyed) {
+        return;
+      }
+
+      const callback: Highcharts.ChartCallbackFunction = (chart: Highcharts.Chart) => {
+        if (chart.renderer.forExport || this.isDestroyed) return;
+        return this.chartInstance.emit(chart);
+      };
+
+      const chart = this.getChartFactory(highcharts, constructorType)(
+        this.el.nativeElement,
+        // Read options without tracking them here: option changes should update
+        // the existing chart, not tear it down and create a new one.
+        untracked(() => this.options()),
+        callback,
+      );
+
+      this.chart.set(chart);
+
+      onCleanup(() => {
+        if (this.chart() === chart) {
+          this.chart.set(null);
+        }
+
+        chart.destroy();
+      });
+    });
+  }
 
   private keepChartUpToDate(): void {
-    effect(async () => {
+    let lastChart: Highcharts.Chart | null = null;
+
+    effect(() => {
+      const chart = this.chart();
+      // Deprecated inputs remain supported internally until they are removed.
+      // eslint-disable-next-line @typescript-eslint/no-deprecated
       const update = this.update();
+      // eslint-disable-next-line @typescript-eslint/no-deprecated
       const oneToOne = this.oneToOne();
       const options = this.options();
-      this._chartInstance = await this.chart();
-      if (!this.chartCreated) {
-        if (this._chartInstance) {
-          this.chartCreated = true;
+
+      if (!chart) {
+        return;
+      }
+
+      // Skip the first pass after creation. The constructor already consumed
+      // the initial options, so calling `update` immediately would duplicate work.
+      if (chart !== lastChart) {
+        lastChart = chart;
+        return;
+      }
+
+      if (update) {
+        chart.update(options, true, oneToOne);
+      }
+    });
+  }
+
+  private async initializeHighcharts(): Promise<void> {
+    await this.pendingTasks.run(async () => {
+      try {
+        const highcharts = await this.highchartsChartService.load(this.relativeConfig);
+        const delayMs = this.relativeConfig?.timeout ?? this.timeout ?? 0;
+
+        // Optional escape hatch: defer chart creation after Highcharts is ready
+        // (e.g. to let the container settle). Skipped entirely by default.
+        if (delayMs > 0) {
+          await this.delay(delayMs);
         }
-      } else {
-        if (update) {
-          this._chartInstance?.update(options, true, oneToOne);
+
+        if (!this.isDestroyed) {
+          this.loadedHighcharts.set(highcharts);
         }
+      } catch (error) {
+        // Core/module loading failed: leave the chart uncreated and surface the
+        // reason instead of letting it become an unhandled promise rejection.
+        console.error('Highcharts failed to load; chart was not created.', error);
       }
     });
   }
 
   public constructor() {
-    // should stop loading on the server side for SSR
     if (this.platformId && isPlatformServer(this.platformId)) {
       return;
     }
-    // make sure to load global config + modules on demand
-    this.highchartsChartService.load(this.relativeConfig);
-    // destroy the chart when the directive is destroyed
+
     this.destroyRef.onDestroy(() => {
-      this._chartInstance?.destroy();
-      this._chartInstance = undefined;
       this.isDestroyed = true;
-    }); // #44
+    });
 
-    // Keep the chart up to date whenever options change or the update special input is set to true
+    this.createChart();
     this.keepChartUpToDate();
+    void this.initializeHighcharts();
   }
 }