fix(store): hydrate initial chart metadata (#519)

Hydrate the untouched initial visualization when asynchronous metadata first arrives while preserving existing user charts and undo history.

Fixes #189

Author: ObservedObserver

packages/graphic-walker/src/store/index.tsx

@@ -1,4 +1,4 @@
-import React, { useContext, useMemo, useEffect, createContext, useRef } from 'react';
+import React, { useContext, useMemo, useEffect, createContext } from 'react';
 import { VizSpecStore } from './visualSpecStore';
 import { IComputationFunction, IDefaultConfig, IMutField, IRow } from '../interfaces';
 
@@ -51,33 +51,25 @@ export const VizStoreWrapper = (props: VizStoreWrapperProps) => {
         return getVizStore(storeKey, props.meta, { onMetaChange: props.onMetaChange, defaultConfig });
     // IMPORTANT the store is only associated with the storeKey
     }, [storeKey]);
-    const lastMeta = useRef(props.meta);
+
+    // Keep this effect before metadata sync: first hydration reads the current
+    // default config when rebuilding the untouched loading placeholder.
     useEffect(() => {
-        if (lastMeta.current !== props.meta) {
-            store.setMeta(props.meta);
-            lastMeta.current = props.meta;
-        }
-    }, [props.meta, store]);
-    const lastOnMetaChange = useRef(props.onMetaChange);
+        const defaultConfig = props.defaultRenderer
+            ? { ...props.defaultConfig, layout: { renderer: props.defaultRenderer, ...(props.defaultConfig?.layout ?? {}) } }
+            : props.defaultConfig;
+        store.setDefaultConfig(defaultConfig);
+    }, [props.defaultConfig, props.defaultRenderer, store]);
+
     useEffect(() => {
-        if (lastOnMetaChange.current !== props.onMetaChange) {
-            store.setOnMetaChange(props.onMetaChange);
-            lastOnMetaChange.current = props.onMetaChange;
-        }
+        // Always sync once on mount so a keepAlive store cannot retain stale
+        // loading metadata across an unmount/remount boundary.
+        store.setMeta(props.meta);
     }, [props.meta, store]);
 
-    const lastDefaultConfig = useRef(props.defaultConfig);
-    const lastDefaultRenderer = useRef(props.defaultRenderer);
     useEffect(() => {
-        if (lastDefaultConfig.current !== props.defaultConfig || lastDefaultRenderer.current !== props.defaultRenderer) {
-            const defaultConfig = props.defaultRenderer
-                ? { ...props.defaultConfig, layout: { renderer: props.defaultRenderer, ...(props.defaultConfig?.layout ?? {}) } }
-                : props.defaultConfig;
-            store.setDefaultConfig(defaultConfig);
-            lastDefaultConfig.current = props.defaultConfig;
-            lastDefaultRenderer.current = props.defaultRenderer;
-        }
-    }, [props.defaultConfig, props.defaultRenderer, store]);
+        store.setOnMetaChange(props.onMetaChange);
+    }, [props.onMetaChange, store]);
 
     useEffect(() => {
         if (props.storeRef) {

packages/graphic-walker/src/store/visualSpecStore.test.ts

@@ -0,0 +1,178 @@
+import type { IMutField } from '../interfaces';
+import { newChart } from '../models/visSpecHistory';
+import { VizSpecStore } from './visualSpecStore';
+
+const META: IMutField[] = [
+    { fid: 'category', name: 'Category', semanticType: 'nominal', analyticType: 'dimension' },
+    { fid: 'sales', name: 'Sales', semanticType: 'quantitative', analyticType: 'measure' },
+];
+
+const dispatchEvent = jest.fn();
+const originalDocument = Object.getOwnPropertyDescriptor(globalThis, 'document');
+const originalCustomEvent = Object.getOwnPropertyDescriptor(globalThis, 'CustomEvent');
+
+const metaFieldIds = (store: VizSpecStore) => {
+    const expectedIds = new Set(META.map((field) => field.fid));
+    return [...store.currentEncodings.dimensions, ...store.currentEncodings.measures]
+        .map((field) => field.fid)
+        .filter((fid) => expectedIds.has(fid));
+};
+
+describe('VizSpecStore metadata updates', () => {
+    beforeAll(() => {
+        Object.defineProperty(globalThis, 'document', {
+            configurable: true,
+            value: { dispatchEvent },
+        });
+        Object.defineProperty(globalThis, 'CustomEvent', {
+            configurable: true,
+            value: class {
+                constructor(
+                    public type: string,
+                    public init?: CustomEventInit,
+                ) {}
+            },
+        });
+    });
+
+    beforeEach(() => {
+        dispatchEvent.mockClear();
+    });
+
+    afterAll(() => {
+        if (originalDocument) {
+            Object.defineProperty(globalThis, 'document', originalDocument);
+        } else {
+            Reflect.deleteProperty(globalThis, 'document');
+        }
+        if (originalCustomEvent) {
+            Object.defineProperty(globalThis, 'CustomEvent', originalCustomEvent);
+        } else {
+            Reflect.deleteProperty(globalThis, 'CustomEvent');
+        }
+    });
+
+    test('hydrates the initial chart when asynchronous metadata first arrives', () => {
+        const store = new VizSpecStore([]);
+        const initialVisId = store.currentVis.visId;
+
+        // App.tsx creates a fresh [] while async fields are still unavailable.
+        store.setMeta([]);
+        store.setMeta([]);
+        store.setMeta(META);
+        expect(dispatchEvent).toHaveBeenCalledTimes(1);
+        expect(dispatchEvent.mock.calls[0][0]).toMatchObject({ type: 'edit-graphic-walker' });
+        expect(store.canUndo).toBe(false);
+        const chartOneFieldsAfterHydration = metaFieldIds(store);
+
+        store.addVisualization();
+        const chartTwoFields = metaFieldIds(store);
+
+        store.selectVisualization(0);
+
+        expect({
+            chartOneFieldsAfterHydration,
+            chartTwoFields,
+            chartOneFieldsAfterSwitchingBack: metaFieldIds(store),
+        }).toEqual({
+            chartOneFieldsAfterHydration: ['category', 'sales'],
+            chartTwoFields: ['category', 'sales'],
+            chartOneFieldsAfterSwitchingBack: ['category', 'sales'],
+        });
+        expect(store.currentVis.visId).toBe(initialVisId);
+    });
+
+    test('preserves an existing user chart when metadata props update', () => {
+        const store = new VizSpecStore(META);
+        store.moveField('dimensions', 0, 'columns', 0);
+        const editedChart = store.currentVis;
+        const updatedMeta: IMutField[] = [
+            ...META,
+            { fid: 'profit', name: 'Profit', semanticType: 'quantitative', analyticType: 'measure' },
+        ];
+
+        store.setMeta(updatedMeta);
+
+        expect(store.meta).toEqual(updatedMeta);
+        expect(store.currentVis).toBe(editedChart);
+        expect(store.currentEncodings.columns.map((field) => field.fid)).toEqual(['category']);
+    });
+
+    test('preserves a chart edited before asynchronous metadata arrives', () => {
+        const store = new VizSpecStore([]);
+        store.setVisualConfig('geoms', ['line']);
+        const editedChart = store.currentVis;
+
+        store.setMeta(META);
+
+        expect(store.currentVis).toBe(editedChart);
+        expect(store.currentVis.config.geoms).toEqual(['line']);
+        expect(metaFieldIds(store)).toEqual([]);
+    });
+
+    test('preserves a supplied chart when asynchronous metadata arrives', () => {
+        const store = new VizSpecStore([]);
+        store.replaceNow(newChart(META, 'Supplied chart', 'supplied-vis-id'));
+        const suppliedChart = store.currentVis;
+
+        store.setMeta(META);
+
+        expect(store.currentVis).toBe(suppliedChart);
+        expect(store.currentVis.visId).toBe('supplied-vis-id');
+        expect(metaFieldIds(store)).toEqual(['category', 'sales']);
+    });
+
+    test('preserves multiple charts created before asynchronous metadata arrives', () => {
+        const store = new VizSpecStore([]);
+        const firstChart = store.visList[0];
+        store.addVisualization();
+        const secondChart = store.visList[1];
+
+        store.setMeta(META);
+
+        expect(store.visList[0]).toBe(firstChart);
+        expect(store.visList[1]).toBe(secondChart);
+    });
+
+    test('hydrates a reset placeholder when metadata arrives', () => {
+        const store = new VizSpecStore([]);
+        store.resetVisualization('Reset while loading');
+        const resetVisId = store.currentVis.visId;
+
+        store.setMeta(META);
+
+        expect(store.currentVis.name).toBe('Reset while loading');
+        expect(store.currentVis.visId).toBe(resetVisId);
+        expect(metaFieldIds(store)).toEqual(['category', 'sales']);
+    });
+
+    test('hydrates the remaining pristine placeholder after the first chart is removed', () => {
+        const store = new VizSpecStore([]);
+        store.addVisualization();
+        const remainingVisId = store.visList[1].now.visId;
+        store.removeVisualization(0);
+
+        store.setMeta(META);
+
+        expect(store.currentVis.visId).toBe(remainingVisId);
+        expect(metaFieldIds(store)).toEqual(['category', 'sales']);
+    });
+
+    test('keeps an explicitly empty store chartless when metadata arrives', () => {
+        const store = new VizSpecStore([], { empty: true });
+
+        store.setMeta(META);
+
+        expect(store.visLength).toBe(0);
+        expect(store.meta).toEqual(META);
+    });
+
+    test('uses the latest default config when hydrating the initial chart', () => {
+        const store = new VizSpecStore([]);
+        store.setDefaultConfig({ layout: { renderer: 'observable-plot' } });
+
+        store.setMeta(META);
+
+        expect(store.currentVis.layout.renderer).toBe('observable-plot');
+    });
+});

packages/graphic-walker/src/store/visualSpecStore.ts

@@ -14,6 +14,7 @@ import {
     redo,
     undo,
 } from '../models/visSpecHistory';
+import { create } from '../models/withHistory';
 import { emptyEncodings, forwardVisualConfigs, visSpecDecoder } from '../utils/save';
 import { feature } from 'topojson-client';
 import type { FeatureCollection } from 'geojson';
@@ -62,6 +63,7 @@ const disposerRegister = (typeof FinalizationRegistry === 'undefined' ? null : n
 export class VizSpecStore {
     instanceID: string = uniqueId();
     visList: VisSpecWithHistory[];
+    private pristineInitialCharts: Set<VisSpecWithHistory>;
     visIndex: number = 0;
     createdVis: number = 0;
     editingFilterIdx: number | null = null;
@@ -107,11 +109,15 @@ export class VizSpecStore {
     ) {
         this.meta = meta;
         this.visList = options?.empty ? [] : [fromFields(meta, 'Chart 1', options?.defaultConfig)];
+        this.pristineInitialCharts = meta.length === 0 ? new Set(this.visList) : new Set();
         this.createdVis = this.visList.length;
         this.defaultConfig = options?.defaultConfig;
         this.onMetaChange = options?.onMetaChange;
-        makeAutoObservable(this, {
+        makeAutoObservable<this, 'pristineInitialCharts'>(this, {
             visList: observable.shallow,
+            // Identity distinguishes the untouched async-loading placeholder from
+            // imported or edited charts; it must not be wrapped by MobX.
+            pristineInitialCharts: false,
             allEncodings: computed.struct,
             filters: observable.ref,
             tableCollapsedHeaderMap: observable.ref,
@@ -388,7 +394,16 @@ export class VizSpecStore {
     }
 
     setMeta(meta: IMutField[]) {
+        const pristineInitialChart =
+            this.visList.length === 1 && this.pristineInitialCharts.has(this.visList[0]) ? this.visList[0] : undefined;
         this.meta = meta;
+        if (meta.length === 0) return;
+
+        this.pristineInitialCharts.clear();
+        if (pristineInitialChart) {
+            const { name, visId } = pristineInitialChart.now;
+            this.visList[0] = create(newChart(meta, name ?? 'Chart 1', visId, this.defaultConfig));
+        }
     }
 
     setOnMetaChange(onMetaChange?: (fid: string, diffMeta: Partial<IMutField>) => void) {
@@ -400,19 +415,29 @@ export class VizSpecStore {
     }
 
     resetVisualization(name = 'Chart 1') {
+        const trackAsInitialPlaceholder = this.meta.length === 0 && this.pristineInitialCharts.size > 0;
         this.visList = [fromFields(this.meta, name, this.defaultConfig)];
+        if (trackAsInitialPlaceholder) {
+            this.pristineInitialCharts.clear();
+            this.pristineInitialCharts.add(this.visList[0]);
+        }
         this.createdVis = 1;
     }
 
     addVisualization(defaultName?: string | ((index: number) => string)) {
+        const trackAsInitialPlaceholder = this.meta.length === 0 && this.pristineInitialCharts.size > 0;
         const name = defaultName ? (typeof defaultName === 'function' ? defaultName(this.createdVis + 1) : defaultName) : 'Chart ' + (this.createdVis + 1);
         this.visList.push(fromFields(this.meta, name, this.defaultConfig));
+        if (trackAsInitialPlaceholder) {
+            this.pristineInitialCharts.add(this.visList[this.visList.length - 1]);
+        }
         this.createdVis += 1;
         this.visIndex = this.visList.length - 1;
     }
 
     removeVisualization(index: number) {
         if (this.visLength === 1) return;
+        this.pristineInitialCharts.delete(this.visList[index]);
         if (this.visIndex >= index && this.visIndex > 0) this.visIndex -= 1;
         this.visList.splice(index, 1);
     }