diff --git a/.changeset/react-cells-inline-selector.md b/.changeset/react-cells-inline-selector.md new file mode 100644 index 0000000000..fc12f1aafe --- /dev/null +++ b/.changeset/react-cells-inline-selector.md @@ -0,0 +1,5 @@ +--- +"@joint/react": patch +--- + +useCells - fix an inline selector that captures a changed prop returning a stale value until the next graph change diff --git a/.changeset/react-elements-measured-app-resize.md b/.changeset/react-elements-measured-app-resize.md index 9ed255334c..6c5ef0a59c 100644 --- a/.changeset/react-elements-measured-app-resize.md +++ b/.changeset/react-elements-measured-app-resize.md @@ -2,4 +2,4 @@ "@joint/react": patch --- -useOnElementsMeasured - fix to fire exactly once per settled change, when no element is still waiting to be measured +useOnElementsMeasured - fix to fire exactly once per settled change (an addition, a removal or a re-measurement), when no element is still waiting to be measured diff --git a/.changeset/react-store-callbacks.md b/.changeset/react-store-callbacks.md new file mode 100644 index 0000000000..0103451a05 --- /dev/null +++ b/.changeset/react-store-callbacks.md @@ -0,0 +1,5 @@ +--- +"@joint/react": patch +--- + + - fix a store callback that throws or changes the graph breaking later updates diff --git a/packages/joint-react/src/components/graph/graph-provider.tsx b/packages/joint-react/src/components/graph/graph-provider.tsx index 7556ab9dcc..a08a752b74 100644 --- a/packages/joint-react/src/components/graph/graph-provider.tsx +++ b/packages/joint-react/src/components/graph/graph-provider.tsx @@ -3,7 +3,7 @@ import React, { memo, useLayoutEffect, useRef } from 'react'; import { useImperativeApi } from '../../hooks/use-imperative-api'; import { GraphStoreContext } from '../../context'; import { GraphStore } from '../../store'; -import type { AutoSizeOrigin } from '../../store/graph-store'; +import type { AutoSizeOrigin } from '../../store/measurement'; import type { OnIncrementalCellsChange } from '../../store/graph-projection'; import type { ElementJSONInit, LinkJSONInit, CellInput } from '../../types/cell.types'; diff --git a/packages/joint-react/src/components/paper/render-element/__tests__/paper-element-item.test.tsx b/packages/joint-react/src/components/paper/render-element/__tests__/paper-element-item.test.tsx index 8acc8e395d..4556108a9c 100644 --- a/packages/joint-react/src/components/paper/render-element/__tests__/paper-element-item.test.tsx +++ b/packages/joint-react/src/components/paper/render-element/__tests__/paper-element-item.test.tsx @@ -10,7 +10,9 @@ import { CellIdContext, GraphStoreContext, PaperStoreContext } from '../../../.. import { ELEMENT_MODEL_TYPE } from '../../../../mvc/element-model'; import type { CellRecord, CellId } from '../../../../types/cell.types'; -const RenderEmpty: ComponentType> = () => ; +const RenderEmpty: ComponentType> = () => ( + +); const CELLS: readonly CellRecord[] = [ { @@ -27,7 +29,11 @@ const CELLS: readonly CellRecord[] = [ * can re-mount SVG / HTML element items with `portalElement={null}` for the * defensive guard branches. */ -function StoreCapture({ onCapture }: { readonly onCapture: (graph: unknown, paper: unknown) => void }) { +function StoreCapture({ + onCapture, +}: { + readonly onCapture: (graph: unknown, paper: unknown) => void; +}) { const graphStore = useContext(GraphStoreContext); const paperStore = useContext(PaperStoreContext); if (graphStore && paperStore) onCapture(graphStore, paperStore); @@ -63,11 +69,7 @@ describe('paper-element-item exports', () => { value={capturedPaper as React.ContextType} > - + @@ -101,11 +103,7 @@ describe('paper-element-item exports', () => { value={capturedPaper as React.ContextType} > - + @@ -143,18 +141,16 @@ describe('paper-element-item exports', () => { value={capturedPaper as React.ContextType} > - + ); // Placeholder wrapper should still be created with id and zero geometry. - const wrapper = portalTarget.querySelector('div[model-id="missing-cell-id"]') as HTMLDivElement | null; + const wrapper = portalTarget.querySelector( + 'div[model-id="missing-cell-id"]' + ) as HTMLDivElement | null; expect(wrapper).toBeTruthy(); expect(wrapper?.style.width).toBe('0px'); expect(wrapper?.style.height).toBe('0px'); diff --git a/packages/joint-react/src/components/paper/render-element/paper-element-item.tsx b/packages/joint-react/src/components/paper/render-element/paper-element-item.tsx index 8c77af7460..e4341e65a0 100644 --- a/packages/joint-react/src/components/paper/render-element/paper-element-item.tsx +++ b/packages/joint-react/src/components/paper/render-element/paper-element-item.tsx @@ -1,9 +1,4 @@ -import { - useLayoutEffect, - useMemo, - type ComponentType, - type CSSProperties, -} from 'react'; +import { useLayoutEffect, useMemo, type ComponentType, type CSSProperties } from 'react'; import { createPortal } from 'react-dom'; import typedMemo from '../../../utils/typed-react'; import { useGraphStore } from '../../../hooks/use-graph-store'; @@ -27,20 +22,18 @@ export interface ElementItemProps { readonly renderElement: ComponentType>; /** The DOM element to portal into. */ readonly portalElement: SVGElement | HTMLElement | null; - /** Whether all auto-sized elements have been measured. */ - readonly areElementsMeasured: boolean; } /** * SVG element portal component. Subscribes only to the element's `data` * slice, position and size are handled by JointJS's view transform and - * never cause a React re-render here. Clears cached views after - * measurement to force re-render with correct dimensions. + * never cause a React re-render here. Clears the cached view once its content + * is committed, so links resolve against the rendered magnets. * @param props - render/portal props * @internal */ function SVGElementItemComponent(props: ElementItemProps) { - const { renderElement: RenderElement, portalElement, areElementsMeasured } = props; + const { renderElement: RenderElement, portalElement } = props; const id = useCellId(); // Subscribe to just this element's `data` slice (missing-tolerant — the portal // can mount before the record lands in the store, and briefly after removal). @@ -54,13 +47,13 @@ function SVGElementItemComponent(props: ElementItemProps) { // inside `renderElement` has already registered with the size observer // (in HTML overlay mode this item follows `HTMLElementItem`, which renders // the user content, in sibling order). O(1), and a no-op on re-runs. - graphStore.markElementRendered(id); + graphStore.measurement.markRendered(id); if (!paper) return; graphStore.clearViewForElementAndLinks({ cellId: id, paper, }); - }, [id, graphStore, areElementsMeasured, paper]); + }, [id, graphStore, paper]); if (!portalElement) { return null; diff --git a/packages/joint-react/src/hooks/__tests__/use-cells.test.tsx b/packages/joint-react/src/hooks/__tests__/use-cells.test.tsx index 69593db8d4..ee6cc6fc84 100644 --- a/packages/joint-react/src/hooks/__tests__/use-cells.test.tsx +++ b/packages/joint-react/src/hooks/__tests__/use-cells.test.tsx @@ -1310,3 +1310,78 @@ describe('useCells (selector reactivity — no collection)', () => { expect(renderedIds).toEqual(['a', 'b']); }); }); + +describe('useCells — what a re-render changes', () => { + const element = (id: string | number, x: number): CellRecord => + ({ + id, + type: ELEMENT_MODEL_TYPE, + position: { x, y: 0 }, + size: { width: 10, height: 10 }, + }) as CellRecord; + const cells: readonly CellRecord[] = [element(1, 100), element('1', 200), element('a|b', 300)]; + const selectXs = (picked: readonly CellRecord[]) => + picked.map((cell) => (cell as { readonly position?: { readonly x: number } }).position?.x); + function cellsWrapper({ children }: { readonly children: React.ReactNode }) { + return {children}; + } + + // Regression: ids were joined into the key that says "same target", so the + // number 1 and the string '1' were one target and the hook kept the first. + it('tells a numeric id from the same id as a string', () => { + const { result, rerender } = renderHook( + ({ ids }: { readonly ids: ReadonlyArray }) => useCells(ids, selectXs), + { wrapper: cellsWrapper, initialProps: { ids: [1] as ReadonlyArray } } + ); + expect(result.current).toEqual([100]); + + rerender({ ids: ['1'] }); + + expect(result.current).toEqual([200]); + }); + + // Regression: the selector was read through a ref and run on a store change + // only, so an inline selector returned what the previous props selected. + it('runs an inline selector that captured a changed prop, without a graph change', () => { + const { result, rerender } = renderHook( + ({ minimumX }: { readonly minimumX: number }) => + useCells( + (all) => + all.filter( + (cell) => + ((cell as { readonly position?: { readonly x: number } }).position?.x ?? 0) >= + minimumX + ).length + ), + { wrapper: cellsWrapper, initialProps: { minimumX: 0 } } + ); + expect(result.current).toBe(3); + + rerender({ minimumX: 250 }); + + expect(result.current).toBe(1); + }); +}); + +describe('useCells dev warnings', () => { + // Regression: the warning named `useGraphState`, which useCells runs on, not + // the hook the application called. + it('names useCells when its selector returns new objects on every run', async () => { + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const { result } = renderHook( + () => { + useCells((cells) => cells.map((cell) => ({ id: cell.id }))); + return useGraphStore().graph; + }, + { wrapper } + ); + + await act(async () => { + (result.current.getCell('a') as dia.Element).position(5, 0); + await flush(); + }); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('[useCells] Selector returns')); + warn.mockRestore(); + }); +}); diff --git a/packages/joint-react/src/hooks/__tests__/use-graph-state.test.tsx b/packages/joint-react/src/hooks/__tests__/use-graph-state.test.tsx new file mode 100644 index 0000000000..c0e0adcd1b --- /dev/null +++ b/packages/joint-react/src/hooks/__tests__/use-graph-state.test.tsx @@ -0,0 +1,328 @@ +import React, { Suspense, startTransition, useState } from 'react'; +import { render, renderHook, act, screen } from '@testing-library/react'; +import { dia } from '@joint/core'; +import { GraphProvider } from '../../components/graph/graph-provider'; +import { useGraphState } from '../use-graph-state'; +import { useGraphStore } from '../use-graph-store'; +import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model'; +import { DEFAULT_CELL_NAMESPACE } from '../../store/graph-store'; +import type { GraphState } from '../../store/graph-state'; +import type { CellRecord } from '../../types/cell.types'; + +const element = (id: string, x = 0): CellRecord => + ({ + id, + type: ELEMENT_MODEL_TYPE, + position: { x, y: 0 }, + size: { width: 10, height: 10 }, + }) as CellRecord; + +const initialCells: readonly CellRecord[] = [element('a'), element('b', 50)]; + +function wrapper({ children }: { readonly children: React.ReactNode }) { + return {children}; +} + +const flush = () => new Promise((resolve) => queueMicrotask(resolve)); +/** Runs a graph mutation and drains the store's microtask commit. */ +const commit = (run: () => void) => + act(async () => { + run(); + await flush(); + }); + +const selectGraphSizingVersion = (state: GraphState) => state.sizingVersion; +const selectIsGraphSizingComplete = (state: GraphState) => state.sizingVersion > 0; +const selectCount = (state: GraphState) => state.cells.length; +const selectIds = (state: GraphState) => state.cells.map((cell) => cell.id); +const isSameParity = (a: number, b: number) => a % 2 === b % 2; +const selectXOf = (id: string) => (state: GraphState) => state.getCell(id)?.position?.x; +const selectAX = selectXOf('a'); +// Custom selectors derived from the shipped ones: plain functions compose. +const selectHasSettledTwice = (state: GraphState) => selectGraphSizingVersion(state) > 1; +const selectMeasuredCount = (state: GraphState) => + selectIsGraphSizingComplete(state) ? state.cells.length : 0; + +/** Mounts a hook next to a handle on the graph, counting renders. */ +function mount(useHook: () => Result) { + let renderCount = 0; + const view = renderHook( + () => { + renderCount += 1; + return { value: useHook(), graph: useGraphStore().graph }; + }, + { wrapper } + ); + const getElement = (id: string) => view.result.current.graph.getCell(id) as dia.Element; + return { + ...view, + value: () => view.result.current.value, + graph: () => view.result.current.graph, + getElement, + renders: () => renderCount, + }; +} + + +describe('useGraphState', () => { + it('selects from the cells and re-renders when the selected value changes', async () => { + const { value, graph } = mount(() => useGraphState(selectCount)); + expect(value()).toBe(2); + + await commit(() => graph().addCell(element('c'))); + + expect(value()).toBe(3); + }); + + it('does not re-render for a change that leaves the selected value equal', async () => { + const { getElement, renders } = mount(() => useGraphState(selectCount)); + const rendersBefore = renders(); + + await commit(() => getElement('a').position(30, 30)); + + expect(renders()).toBe(rendersBefore); + }); + + it('reads one cell without running its selector when another cell changes', async () => { + const selector = jest.fn(selectAX); + const { value, getElement, renders } = mount(() => useGraphState(selector)); + const rendersBefore = renders(); + const callsBefore = selector.mock.calls.length; + + await commit(() => getElement('b').position(70, 0)); + expect(selector).toHaveBeenCalledTimes(callsBefore); + expect(renders()).toBe(rendersBefore); + + await commit(() => getElement('a').position(30, 0)); + expect(value()).toBe(30); + }); + + it('keeps the selected reference stable across re-renders and equal results', async () => { + const { value, getElement, rerender } = mount(() => useGraphState(selectIds)); + const before = value(); + + rerender(); + await commit(() => getElement('a').position(30, 0)); + + expect(value()).toBe(before); + }); + + it('uses the custom equality to decide what a change is', async () => { + const { value, graph } = mount(() => useGraphState(selectCount, isSameParity)); + + await commit(() => graph().addCells([element('c'), element('d')])); + expect(value()).toBe(2); + + await commit(() => graph().addCell(element('e'))); + expect(value()).toBe(5); + }); + + // The selector is part of what the hook renders: a new one (a changed prop + // captured by an inline selector) is read at once, not on the next store change. + it('runs a new selector on re-render, without a store change', () => { + const { result } = renderHook( + () => { + const [id, setId] = useState('a'); + return { x: useGraphState((state) => state.getCell(id)?.position?.x), setId }; + }, + { wrapper } + ); + expect(result.current.x).toBe(0); + + act(() => result.current.setId('b')); + + expect(result.current.x).toBe(50); + }); + + it('follows the cell a re-rendered selector reads, and drops the previous one', async () => { + const { result } = renderHook( + () => { + const [id, setId] = useState('a'); + const { graph } = useGraphStore(); + return { x: useGraphState((state) => state.getCell(id)?.position?.x), setId, graph }; + }, + { wrapper } + ); + act(() => result.current.setId('b')); + + await commit(() => (result.current.graph.getCell('b') as dia.Element).position(70, 0)); + + expect(result.current.x).toBe(70); + }); + + // Regression: a consumer mounted after the graph settled has to see the + // settled state on its first render, with no change to wake it. + it('reads the current state when mounted after the graph settled', async () => { + const { result } = renderHook( + () => { + const [isShown, setIsShown] = useState(false); + return { isShown, setIsShown }; + }, + { wrapper } + ); + await act(async () => flush()); + expect(result.current.isShown).toBe(false); + + const late = mount(() => useGraphState(selectIsGraphSizingComplete)); + await act(async () => flush()); + + expect(late.value()).toBe(true); + }); + + describe('sizing version', () => { + it('re-renders its reader once per settled change, not per commit', async () => { + const { value, getElement, graph, renders } = mount(() => + useGraphState(selectGraphSizingVersion) + ); + await act(async () => flush()); + expect(value()).toBe(1); + const rendersBefore = renders(); + + // A move is a commit, but not a measurement. + await commit(() => getElement('a').position(9, 9)); + expect(value()).toBe(1); + expect(renders()).toBe(rendersBefore); + + await commit(() => graph().addCell(element('c'))); + expect(value()).toBe(2); + }); + + it('selectIsGraphSizingComplete turns true once sizes settle and re-renders its reader only then', async () => { + const { value, graph, renders } = mount(() => useGraphState(selectIsGraphSizingComplete)); + await act(async () => flush()); + expect(value()).toBe(true); + const rendersBefore = renders(); + + // A later settled change bumps the sizing version, but the diagram stays measured. + await commit(() => graph().addCell(element('c'))); + expect(value()).toBe(true); + expect(renders()).toBe(rendersBefore); + + // A reset to a diagram without sizes is not measured. + await commit(() => graph().resetCells([])); + expect(value()).toBe(false); + }); + + it('is measured for an external graph that already holds elements', async () => { + const graph = new dia.Graph({}, { cellNamespace: DEFAULT_CELL_NAMESPACE }); + graph.addCells([element('a'), element('b')] as never); + const { result } = renderHook(() => useGraphState(selectIsGraphSizingComplete), { + wrapper: ({ children }: { readonly children: React.ReactNode }) => ( + {children} + ), + }); + await act(async () => flush()); + expect(result.current).toBe(true); + + await commit(() => graph.getCell('a').remove()); + expect(result.current).toBe(true); + }); + + // A selector derived from a shipped one is a selector like any other. + it('re-renders a custom selector derived from selectGraphSizingVersion only when its value changes', async () => { + const { value, graph, renders } = mount(() => useGraphState(selectHasSettledTwice)); + await act(async () => flush()); + expect(value()).toBe(false); + const rendersBefore = renders(); + + await commit(() => graph().addCell(element('c'))); + expect(value()).toBe(true); + const rendersAfterChange = renders(); + expect(rendersAfterChange).toBeGreaterThan(rendersBefore); + + // Settles a third time: the sizing version changes, the derived value does not. + await commit(() => graph().addCell(element('d'))); + expect(value()).toBe(true); + expect(renders()).toBe(rendersAfterChange); + }); + + it('combines the sizing version with the cells in one selector', async () => { + const { value, graph } = mount(() => useGraphState(selectMeasuredCount)); + await act(async () => flush()); + expect(value()).toBe(2); + + await commit(() => graph().addCell(element('c'))); + expect(value()).toBe(3); + + await commit(() => graph().resetCells([])); + expect(value()).toBe(0); + }); + + // Regression: read through the cells, a measurement selector made every + // commit rebuild the cells array (O(n) per drag frame). A selector that + // reads only the sizing version is not even run on such a commit. + it.each<[string, () => unknown]>([ + ['useGraphState(selectGraphSizingVersion)', () => useGraphState(selectGraphSizingVersion)], + [ + 'useGraphState(selectIsGraphSizingComplete)', + () => useGraphState(selectIsGraphSizingComplete), + ], + ['useGraphState(a derived selector)', () => useGraphState(selectHasSettledTwice)], + ])('%s does no work on a commit that is not a measurement', async (_name, useReader) => { + const { getElement, renders } = mount(useReader); + await act(async () => flush()); + const rendersBefore = renders(); + const buildArray = jest.spyOn(Array, 'from'); + + for (let frame = 1; frame <= 10; frame += 1) { + await commit(() => getElement('a').position(frame, frame)); + } + + // The container builds its cells array with `Array.from`, and only on demand. + expect(buildArray).not.toHaveBeenCalled(); + expect(renders()).toBe(rendersBefore); + buildArray.mockRestore(); + }); + }); +}); + +// Regression: the subscription followed every read, and React reads while +// rendering. A render it never commits (here a transition that suspends) moved +// the subscription away from the cell the committed tree shows. +describe('useGraphState — a render React throws away', () => { + const NEVER = new Promise(() => {}); + let liveGraph: dia.Graph | undefined; + let startSuspendedTransition: (() => void) | undefined; + + function Suspender({ isSuspended }: Readonly<{ isSuspended: boolean }>) { + // Throwing a promise is how a component suspends. + if (isSuspended) throw NEVER; + return null; + } + function PositionReader({ id }: Readonly<{ id: string }>) { + const x = useGraphState((state: GraphState) => state.getCell(id)?.position?.x); + return {String(x)}; + } + function TransitionApp() { + liveGraph = useGraphStore().graph; + const [id, setId] = useState('a'); + const [isSuspended, setIsSuspended] = useState(false); + startSuspendedTransition = () => + startTransition(() => { + setId('b'); + setIsSuspended(true); + }); + return ( + + + + + ); + } + + it('keeps following the cell the committed tree reads', async () => { + render( + + + + ); + expect(screen.getByTestId('x').textContent).toBe('0'); + + // Renders the reader with `b`, suspends, and keeps showing `a`. + await commit(() => startSuspendedTransition?.()); + expect(screen.getByTestId('x').textContent).toBe('0'); + + await commit(() => (liveGraph?.getCell('a') as dia.Element).position(33, 0)); + expect(screen.getByTestId('x').textContent).toBe('33'); + }); +}); diff --git a/packages/joint-react/src/hooks/__tests__/use-measure-element.test.tsx b/packages/joint-react/src/hooks/__tests__/use-measure-element.test.tsx index e17973f75e..79bb7ef3d5 100644 --- a/packages/joint-react/src/hooks/__tests__/use-measure-element.test.tsx +++ b/packages/joint-react/src/hooks/__tests__/use-measure-element.test.tsx @@ -28,6 +28,7 @@ import { LINK_MODEL_TYPE } from '../../mvc/link-model'; import type { CellRecord } from '../../types/cell.types'; let capturedGraph: dia.Graph | null = null; +let capturedStore: ReturnType | null = null; let capturedPaper: dia.Paper | null = null; const initialCells: readonly CellRecord[] = [ @@ -53,8 +54,10 @@ const initialCells: readonly CellRecord[] = [ function Probe() { const nodeRef = useRef(null); - const { graph } = useGraphStore(); + const store = useGraphStore(); + const { graph } = store; const { paper } = usePaper(); + capturedStore = store; capturedGraph = graph; capturedPaper = paper; const size = useMeasureElement(nodeRef); @@ -135,7 +138,8 @@ describe('useMeasureElement', () => { expect(() => render( - @@ -172,7 +176,11 @@ describe('useMeasureElement', () => { } render( - + ); await new Promise((resolve) => setTimeout(resolve, 50)); @@ -184,7 +192,11 @@ describe('useMeasureElement', () => { it('returns size record without registering when ref.current is null at mount', async () => { const { container } = render( - + ); await waitFor(() => { @@ -193,6 +205,29 @@ describe('useMeasureElement', () => { }); }); + // Regression: the paper can route the links before React commits a measured + // size, so the measured element clears its view once that size is committed. + // This used to be a sweep of every element on a global "measured" flag. + it('clears the view of a measured element when its measured size commits', async () => { + renderProbe(); + await waitFor(() => expect(capturedPaper?.findViewByModel('el')).toBeDefined()); + const clearView = jest.spyOn(capturedStore!, 'clearViewForElementAndLinks'); + const element = capturedGraph!.getCell('el') as dia.Element; + + await act(async () => { + element.position(5, 5); + await Promise.resolve(); + }); + expect(clearView).not.toHaveBeenCalled(); + + await act(async () => { + element.set('size', { width: 120, height: 60 }, { autoSize: true }); + await Promise.resolve(); + }); + const clearedIds = clearView.mock.calls.map(([options]) => options.cellId); + expect(clearedIds).toEqual(['el']); + }); + describe('with a real ResizeObserver mock', () => { // Local ResizeObserver mock so we can deliver entries — the package-level // setup in `__mocks__/jest-setup.ts` installs a stub that never fires. diff --git a/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-events.test.tsx b/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-events.test.tsx index ac86c8f011..94b23c8d09 100644 --- a/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-events.test.tsx +++ b/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-events.test.tsx @@ -1,5 +1,5 @@ /** - * Specification for when `useOnElementsMeasured` delivers an event. + * Specification for when `useOnElementsMeasured` delivers an event (#3520). * * The hook exists so an application can run a layout once element sizes are * known. That only works if one settled change delivers exactly one event: a @@ -26,7 +26,7 @@ import { HTMLHost } from '../../components/html-host'; import { useOnElementsMeasured } from '../use-on-elements-measured'; import { useGraphStore } from '../use-graph-store'; import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model'; -import { AUTO_SIZE_OPTION } from '../../store/graph-store'; +import { AUTO_SIZE_OPTION } from '../../store/measurement'; import type { CellRecord } from '../../types/cell.types'; import type { dia } from '@joint/core'; @@ -121,11 +121,9 @@ function renderGraph(initialCells: CellRecord[]): Harness { function reportMeasurement(graph: dia.Graph, ...ids: string[]) { act(() => { for (const id of ids) { - (graph.getCell(id) as dia.Element).set( - 'size', - { width: 120, height: 40 }, - { [AUTO_SIZE_OPTION]: true } as object - ); + (graph.getCell(id) as dia.Element).set('size', { width: 120, height: 40 }, { + [AUTO_SIZE_OPTION]: true, + } as object); } }); } diff --git a/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-settling.test.tsx b/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-settling.test.tsx index 326e43c1fd..43a507a001 100644 --- a/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-settling.test.tsx +++ b/packages/joint-react/src/hooks/__tests__/use-on-elements-measured-settling.test.tsx @@ -160,6 +160,49 @@ describe('useOnElementsMeasured — a waiting element is removed', () => { }); }); +// Regression: an element that stopped measuring before it was measured stayed +// "waiting" forever and held back every later event. +describe('useOnElementsMeasured — a waiting element stops measuring', () => { + it('delivers the batch, and later changes, once nothing measures the element any more', async () => { + const harness = renderGraph([plain('a')]); + await settleAndClear(harness); + + act(() => { + harness.graph.addCell(pending('b') as never); + }); + await flush(); + expect(harness.events).toHaveLength(0); + + // Its content switches to a plain shape: the measuring node unmounts. + act(() => { + harness.graph.getCell('b').set('data', {}); + }); + await flush(); + expect(harness.events).toHaveLength(1); + + act(() => { + harness.graph.addCell(plain('c') as never); + }); + await flush(); + expect(harness.events).toHaveLength(2); + }); +}); + +// A removal changes what a layout has to arrange, so it is a settled change too. +describe('useOnElementsMeasured — a settled element is removed', () => { + it('delivers one event for the removal', async () => { + const harness = renderGraph([plain('a'), plain('b')]); + await settleAndClear(harness); + + act(() => { + harness.graph.getCell('b').remove(); + }); + await flush(); + + expect(harness.events).toEqual([{ isInitial: false }]); + }); +}); + // The case the hook exists for in a live diagram: `renderElement` renders // something else (a longer label, an expanded card), the node grows, the // ResizeObserver reports the new size and the layout runs again. jsdom has no @@ -194,15 +237,25 @@ describe('useOnElementsMeasured — the content of an element changes', () => { globalThis.ResizeObserver = TestResizeObserver as unknown as typeof ResizeObserver; }); + /** + * The observer holding the node `` registered. The paper observes + * its own host with another `ResizeObserver`, so the instance is found by + * the node: the measured one lives inside the element's `foreignObject`. + */ + function findMeasuredNode() { + for (const observer of TestResizeObserver.instances) { + for (const node of observer.observed) { + if (node.closest('foreignObject')) return { observer, node }; + } + } + throw new Error('no ResizeObserver has the measured node registered'); + } + /** Mounts one measured element, lets it register, and measures it once. */ async function mountMeasured() { const harness = renderGraph([pending('b')]); await flush(); - // StrictMode mounts the store twice; only the live store's observer has - // the node registered by ``. - const observer = TestResizeObserver.instances.find((instance) => instance.observed.size > 0); - if (!observer) throw new Error('no ResizeObserver has the measured node registered'); - const [node] = observer.observed; + const { observer, node } = findMeasuredNode(); act(() => { observer.report(node, 120, 40); @@ -256,9 +309,7 @@ describe('useOnElementsMeasured — the content of an element changes', () => { it('measures an element the application pre-sized to what it will measure', async () => { const harness = renderGraph([pending('b')]); await flush(); - const observer = TestResizeObserver.instances.find((instance) => instance.observed.size > 0); - if (!observer) throw new Error('no ResizeObserver has the measured node registered'); - const [node] = observer.observed; + const { observer, node } = findMeasuredNode(); act(() => { (harness.graph.getCell('b') as dia.Element).resize(120, 40); diff --git a/packages/joint-react/src/hooks/__tests__/use-on-elements-measured.test.tsx b/packages/joint-react/src/hooks/__tests__/use-on-elements-measured.test.tsx index e1256086a0..789389c238 100644 --- a/packages/joint-react/src/hooks/__tests__/use-on-elements-measured.test.tsx +++ b/packages/joint-react/src/hooks/__tests__/use-on-elements-measured.test.tsx @@ -3,7 +3,7 @@ import { renderHook, waitFor, act } from '@testing-library/react'; import { paperRenderElementWrapper } from '../../utils/test-wrappers'; import { useOnElementsMeasured } from '../use-on-elements-measured'; import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model'; -import { AUTO_SIZE_OPTION } from '../../store/graph-store'; +import { AUTO_SIZE_OPTION } from '../../store/measurement'; import { useGraphStore } from '../use-graph-store'; import type { CellRecord } from '../../types/cell.types'; import type { ElementsMeasuredParams } from '../use-on-elements-measured'; @@ -49,12 +49,25 @@ const zeroSizeWrapper = paperRenderElementWrapper({ }, }); -const incrementMeasureState = (previous: number) => previous + 1; - -type MeasureStateRef = ReturnType['measureState']; - -const bumpMeasureFor = (measureState: MeasureStateRef) => () => - measureState.set(incrementMeasureState); +/** + * Mounts the hook for `paperId` through `hookWrapper` and returns a getter of + * the live graph. The wrappers mount their children inside `renderElement`, so + * there is one hook instance per rendered element. + */ +function renderMeasuredProbe( + callback: jest.Mock, + hookWrapper: typeof wrapper = wrapper, + paperId = 'measured-effect-paper' +) { + let graphRef: dia.Graph | undefined; + function Probe() { + graphRef = useGraphStore().graph; + useOnElementsMeasured(paperId, callback); + return null; + } + renderHook(() => Probe(), { wrapper: hookWrapper }); + return () => graphRef as dia.Graph; +} describe('useOnElementsMeasured', () => { it('fires callback with isInitial=true after seed cells are measured', async () => { @@ -66,65 +79,62 @@ describe('useOnElementsMeasured', () => { expect(initialCalls.length).toBeGreaterThan(0); }); - it('subsequent measurement bumps fire callback with isInitial=false', async () => { + it('later settled changes fire the callback with isInitial=false', async () => { const callback = jest.fn(); - let bumpMeasure: () => void = () => {}; - function Probe() { - const { measureState } = useGraphStore(); - bumpMeasure = bumpMeasureFor(measureState); - useOnElementsMeasured('measured-effect-paper', callback); - return null; - } - renderHook(() => Probe(), { wrapper }); + const getGraph = renderMeasuredProbe(callback); await waitFor(() => expect(callback.mock.calls.some(([event]) => event.isInitial === true)).toBe(true) ); callback.mockClear(); act(() => { - bumpMeasure(); + (getGraph().getCell('a') as dia.Element).set('size', { width: 70, height: 70 }, { + [AUTO_SIZE_OPTION]: true, + } as object); }); - await flush(); - expect(callback).toHaveBeenCalled(); + await waitFor(() => expect(callback).toHaveBeenCalled()); for (const [event] of callback.mock.calls) { expect((event as ElementsMeasuredParams).isInitial).toBe(false); } }); - // Regression: ElementModel defaults to size {0,0}. The ResizeObserver - // pipeline sets the real size via `cell.set('size', ..., { autoSize: true })`. - // Previously, the `change:size` listener in graph-changes.ts skipped - // measurement writes, so the measured size never reached the tracking - // logic and `useOnElementsMeasured` never fired for elements that relied - // on DOM measurement (e.g. the flowchart demo). - it('fires callback when elements start at zero size and get measured', async () => { + // An element nothing measures is settled once the paper rendered it, whatever + // its size: zero is a legal size. A later measurement is one more change. + it('fires for a zero-sized element once it is rendered, then for its measurement', async () => { const callback = jest.fn(); - let graphRef: dia.Graph | undefined; + const getGraph = renderMeasuredProbe(callback, zeroSizeWrapper, 'zero-size-paper'); - function Probe() { - const store = useGraphStore(); - graphRef = store.graph; - useOnElementsMeasured('zero-size-paper', callback); - return null; - } + await waitFor(() => expect(callback).toHaveBeenCalled()); + expect(callback.mock.calls[0][0].isInitial).toBe(true); + callback.mockClear(); - renderHook(() => Probe(), { wrapper: zeroSizeWrapper }); + act(() => { + const cell = getGraph().getCell('zero-el') as dia.Element; + cell.set('size', { width: 100, height: 60 }, { [AUTO_SIZE_OPTION]: true } as object); + }); - // Wait for render to complete and graph to be available. - await waitFor(() => expect(graphRef).toBeDefined()); - await flush(); + await waitFor(() => expect(callback).toHaveBeenCalledTimes(1)); + expect(callback.mock.calls[0][0].isInitial).toBe(false); + }); - // Initial size is {0,0} — callback should NOT have fired yet - // (measureState only bumps when elementsMeasured.size > 0). - expect(callback).not.toHaveBeenCalled(); + it('reports isInitial again for the first event after a graph reset', async () => { + const callback = jest.fn(); + const getGraph = renderMeasuredProbe(callback); + await waitFor(() => expect(callback).toHaveBeenCalled()); + callback.mockClear(); - // Simulate ResizeObserver setting the real measured size. act(() => { - const cell = graphRef!.getCell('zero-el') as dia.Element; - cell.set('size', { width: 100, height: 60 }, { [AUTO_SIZE_OPTION]: true } as object); + getGraph().resetCells([ + { + id: 'x', + type: ELEMENT_MODEL_TYPE, + position: { x: 0, y: 0 }, + size: { width: 9, height: 9 }, + }, + ]); }); - await waitFor(() => expect(callback).toHaveBeenCalled()); + await waitFor(() => expect(callback).toHaveBeenCalledTimes(1)); expect(callback.mock.calls[0][0].isInitial).toBe(true); }); @@ -132,7 +142,43 @@ describe('useOnElementsMeasured', () => { // options, so an application's own resize bumped `measureState` like a // measurement write and woke every subscriber. describe('application resizes vs measurement writes', () => { - function renderMeasuredProbe(callback: jest.Mock) { + it('does not fire when the application resizes an element', async () => { + const callback = jest.fn(); + const getGraph = renderMeasuredProbe(callback); + const getElement = () => getGraph().getCell('a') as dia.Element; + await waitFor(() => expect(callback).toHaveBeenCalled()); + callback.mockClear(); + + act(() => { + getElement().resize(80, 80); + }); + await act(async () => flush()); + await act(async () => flush()); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('fires with isInitial=false for a measurement write', async () => { + const callback = jest.fn(); + const getGraph = renderMeasuredProbe(callback); + const getElement = () => getGraph().getCell('a') as dia.Element; + await waitFor(() => expect(callback).toHaveBeenCalled()); + callback.mockClear(); + + act(() => { + getElement().set('size', { width: 80, height: 80 }, { autoSize: true }); + }); + + await waitFor(() => expect(callback).toHaveBeenCalledTimes(1)); + expect(callback.mock.calls[0][0].isInitial).toBe(false); + }); + }); + + // Regression (#3514): the store's `change:size` listener dropped the event + // options, so an application's own resize bumped `measureState` like a + // measurement write and woke every subscriber. + describe('application resizes vs measurement writes', () => { + function renderResizeProbe(callback: jest.Mock) { let graphRef: dia.Graph | undefined; function Probe() { const store = useGraphStore(); @@ -146,7 +192,7 @@ describe('useOnElementsMeasured', () => { it('does not fire when the application resizes an element', async () => { const callback = jest.fn(); - const getElement = renderMeasuredProbe(callback); + const getElement = renderResizeProbe(callback); await waitFor(() => expect(callback).toHaveBeenCalled()); callback.mockClear(); @@ -161,7 +207,7 @@ describe('useOnElementsMeasured', () => { it('fires with isInitial=false for a measurement write', async () => { const callback = jest.fn(); - const getElement = renderMeasuredProbe(callback); + const getElement = renderResizeProbe(callback); await waitFor(() => expect(callback).toHaveBeenCalled()); callback.mockClear(); diff --git a/packages/joint-react/src/hooks/use-are-elements-measured.ts b/packages/joint-react/src/hooks/use-are-elements-measured.ts deleted file mode 100644 index 998d68c011..0000000000 --- a/packages/joint-react/src/hooks/use-are-elements-measured.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { useGraphStore } from './use-graph-store'; -import { useSyncExternalStore } from 'react'; - -/** - * Returns `true` when at least one element has been measured. - */ -export function useAreElementsMeasured() { - const { measureState } = useGraphStore(); - return useSyncExternalStore( - (onStoreChange) => { - // subscribe for both, elements size from container and also measure state changes. - const unsubscribeMeasureState = measureState.subscribe(onStoreChange); - return () => { - unsubscribeMeasureState(); - }; - }, - () => { - return measureState.get() > 0; - } - ); -} diff --git a/packages/joint-react/src/hooks/use-cell.ts b/packages/joint-react/src/hooks/use-cell.ts index 7582bd76b3..25dc5b36f9 100644 --- a/packages/joint-react/src/hooks/use-cell.ts +++ b/packages/joint-react/src/hooks/use-cell.ts @@ -159,8 +159,8 @@ export function useCell | Selected>( diff --git a/packages/joint-react/src/hooks/use-cells-state-selector.ts b/packages/joint-react/src/hooks/use-cells-state-selector.ts new file mode 100644 index 0000000000..da565353e6 --- /dev/null +++ b/packages/joint-react/src/hooks/use-cells-state-selector.ts @@ -0,0 +1,34 @@ +import { useMemo } from 'react'; +import type { AnyCellRecord } from '../types/cell.types'; +import type { GraphStateSelector } from '../store/graph-state'; +import type { AnyCellsSelector, CellsSource, CellsTarget } from './use-cells.types'; + +/** + * The graph state selector of a cells hook: reads what `target` names and + * hands it to `selector`. The same function for as long as the target and the + * selector are. + * @param target - What to read: all cells (`undefined`), no cell (`null`), a cell id, ids or a collection. + * @param targetKey - `toTargetKey(target)`, computed once by the caller. + * @param selector - Derives the value from what was read; the read itself when omitted. + * @returns The selector to pass to `useGraphState`. + * @internal + */ +export function useCellsStateSelector( + target: CellsTarget, + targetKey: unknown, + selector?: AnyCellsSelector +): GraphStateSelector> { + return useMemo( + () => (state) => { + let source: CellsSource; + if (target === undefined) source = state.cells; + else if (target === null) source = undefined; + else if (typeof target === 'object') source = state.getCells(target); + else source = state.getCell(target); + return selector ? selector(source) : source; + }, + // `targetKey` stands in for `target`, see `toTargetKey` in `use-cells.utils`. + // eslint-disable-next-line react-hooks/exhaustive-deps + [targetKey, selector] + ); +} diff --git a/packages/joint-react/src/hooks/use-cells.ts b/packages/joint-react/src/hooks/use-cells.ts index 5f0631785b..6222446688 100644 --- a/packages/joint-react/src/hooks/use-cells.ts +++ b/packages/joint-react/src/hooks/use-cells.ts @@ -1,115 +1,23 @@ -import { useCallback, useMemo, useRef } from 'react'; -import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/with-selector'; import { type mvc, type dia } from '@joint/core'; -import { useGraphStore } from './use-graph-store'; import type { AnyCellRecord, CellId, CellRecord, Computed } from '../types/cell.types'; -import type { ReadonlyContainer } from '../store/state-container'; -import { areArraysShallowEqual, arrayAwareEqual } from '../utils/selector-utils'; -import { isCollection } from '../utils/is'; -import { subscribeToCollection } from '../utils/collection-subscription'; -import { parseUseCellsArgs } from './use-cells.utils'; -import { warnUnstableSelector } from '../utils/dev-warnings'; -import { toCellRecord } from '../state/data-mapping/cell-record-merge'; - -// ── Types ─────────────────────────────────────────────────────────────────── - -/** - * Union of all possible `useCells` return shapes (depends on argument form). - * `Cell` here is the already-resolved (Computed) record the helpers work with. - */ -type CellsResult = - | readonly Cell[] - | Cell - | undefined - | Selected; - -/** Equality function on raw `unknown` values. */ -type UnknownEqual = (a: unknown, b: unknown) => boolean; - -/** Selector over the resolved cells array (the array forms of `useCells`). */ -type CellsSelector = ( - cells: ReadonlyArray> -) => Selected; - -/** Equality test that short-circuits a re-render when the selected value is unchanged. */ -type SelectedEqual = (a: Selected, b: Selected) => boolean; - -// ── Module-scoped helpers ─────────────────────────────────────────────────── - -/** - * Resolves a single cell record for the collection form. Tries the graph - * container first; falls back to the collection itself for cells that exist - * outside the graph (e.g. `ui.Clipboard` clones). Fallback records are cached - * by `dia.Cell` identity so repeat renders share the same reference and the - * outer shallow-equality short-circuit can fire. - */ -function resolveCell( - id: CellId, - container: ReadonlyContainer, - collection: mvc.Collection | undefined, - fallbackCache: WeakMap | undefined -): Cell | undefined { - const fromContainer = container.get(id); - if (fromContainer !== undefined) return fromContainer; - if (!collection || !fallbackCache) return undefined; - const rawCell = collection.get(id); - if (!rawCell) return undefined; - const cached = fallbackCache.get(rawCell); - if (cached !== undefined) return cached as Cell; - const record = toCellRecord(rawCell) as Cell; - fallbackCache.set(rawCell, record); - return record; -} - -/** - * Picks cells named by `subscribedIds` in order. Missing ids (not in the - * graph container and not in the optional fallback `collection`) are skipped. - */ -function pickCells( - container: ReadonlyContainer, - subscribedIds: readonly CellId[], - collection: mvc.Collection | undefined, - fallbackCache: WeakMap | undefined -): readonly Cell[] { - const picked: Cell[] = []; - for (const id of subscribedIds) { - const cell = resolveCell(id, container, collection, fallbackCache); - if (cell !== undefined) picked.push(cell); - } - return picked; -} - -/** - * Builds the next selector result for {@link useCells}. Module-scoped so the - * closure does not nest deeply inside {@link useCells}.select. - */ -function computeNext( - container: ReadonlyContainer, - targetId: CellId | undefined, - subscribedIds: readonly CellId[] | undefined, - collection: mvc.Collection | undefined, - fallbackCache: WeakMap | undefined, - arraySelector: ((cells: readonly Cell[]) => Selected) | undefined, - cellSelector: ((cell: Cell | undefined) => Selected) | undefined -): CellsResult { - // Single-cell-with-selector form: a nullish id resolves to no cell, so the - // selector runs against `undefined` instead of falling through to all cells. - if (cellSelector) { - const cell = - targetId === undefined - ? undefined - : resolveCell(targetId, container, collection, fallbackCache); - return cellSelector(cell); - } - if (targetId !== undefined) { - return resolveCell(targetId, container, collection, fallbackCache); - } - const source: readonly Cell[] = - subscribedIds === undefined - ? container.getSnapshot() - : pickCells(container, subscribedIds, collection, fallbackCache); - return arraySelector ? arraySelector(source) : source; -} +import type { SelectedEqual } from '../store/graph-state'; +import type { AnyCellsSelector, CellsSelector, CellsSource, CellsTarget } from './use-cells.types'; +import { useCellsStateSelector } from './use-cells-state-selector'; +import { toTargetKey } from './use-cells.utils'; +import { useSelectGraphState } from './use-graph-state'; + +export type { SelectedEqual } from '../store/graph-state'; +export type { CellsSelector } from './use-cells.types'; + +/** The arguments of every `useCells` form: an optional target, a selector, an equality function. */ +type CellsArguments = [ + argument1?: CellsTarget | CellsSelector, + argument2?: + | CellsSelector + | ((cell: Computed | undefined) => Selected) + | SelectedEqual, + argument3?: SelectedEqual, +]; // ── Overloads ─────────────────────────────────────────────────────────────── @@ -149,8 +57,8 @@ export function useCells< /** * Subscribe to the full cells array. * - * Returned array reference is stable across data-only mutations (the internal - * container mutates items in-place). Size changes produce a new snapshot token. + * The array is a new reference on every graph change; the records in it keep + * their reference until they change themselves. * @title Subscribe to all cells * @template Cell - input cell record shape (defaults to CellRecord); reads resolve to its Computed form * @returns readonly resolved cells array @@ -261,158 +169,26 @@ export function useCells< export function useCells< Cell extends AnyCellRecord = CellRecord, Selected = ReadonlyArray>, ->( - argument1?: - | CellId - | null - | readonly CellId[] - | CellsSelector - | mvc.Collection, - argument2?: - | CellsSelector - | ((cell: Computed | undefined) => Selected) - | SelectedEqual, - argument3?: SelectedEqual -): CellsResult, Selected> { - const store = useGraphStore(); - // The store holds resolved (Computed) records; the public `Cell` generic is the - // input record shape, so internally we work in `Computed`. - const container = store.graphProjection.cells as ReadonlyContainer>; - - const collectionArgument = isCollection(argument1) ? argument1 : undefined; - - const { targetId, ids, arraySelector, cellSelector, isEqual } = parseUseCellsArgs< - Computed, - Selected - >( - argument1, - argument2, - argument3 - ); - const hasSelector = arraySelector !== undefined || cellSelector !== undefined; - - const arraySelectorRef = useRef(arraySelector); - arraySelectorRef.current = arraySelector; - const cellSelectorRef = useRef(cellSelector); - cellSelectorRef.current = cellSelector; - - /** Local alias for the hook's return shape so the cache type stays readable. */ - type Result = CellsResult, Selected>; - const cachedRef = useRef<{ hasValue: boolean; value: Result }>({ - hasValue: false, - value: undefined, - }); - - const idsKey = useMemo(() => (ids ? ids.join('|') : ''), [ids]); - const idsRef = useRef(ids); - idsRef.current = ids; - - // ── Collection refs (no-op when not in collection form) ── - - const collectionIdsRef = useRef([]); - const collectionVersionRef = useRef(0); - const fallbackCacheRef = useRef>(new WeakMap()); - - // Use `null` sentinel so the init-block runs on first render too, picking up - // any models that already exist in the collection at subscribe time. - const previousCollectionRef = useRef | undefined | null>(null); - if (previousCollectionRef.current !== collectionArgument) { - previousCollectionRef.current = collectionArgument; - collectionIdsRef.current = collectionArgument - ? collectionArgument.models.map((m) => m.id as CellId) - : []; - collectionVersionRef.current++; - } - - // ── Subscribe ── - - const subscribe = useCallback( - (listener: () => void) => { - if (collectionArgument) { - return subscribeToCollection( - collectionArgument, - container, - listener, - collectionIdsRef, - collectionVersionRef - ); - } - if (targetId !== undefined) return container.subscribeById(targetId, listener); - const subscribedIds = idsRef.current; - if (subscribedIds && subscribedIds.length > 0) { - const unsubscribers = subscribedIds.map((id) => container.subscribeById(id, listener)); - return () => { - for (const unsubscribe of unsubscribers) unsubscribe(); - }; - } - return container.subscribe(listener); - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [container, collectionArgument, targetId, idsKey] - ); - - // ── Snapshot ── - - const getSnapshot = useCallback(() => { - // Per-id token for ANY single-cell form (with or without a selector): the - // token is the cell record itself, so the store only re-reads when THIS cell - // changes, never on unrelated commits. Immutable records keep the reference - // stable between changes, so useSyncExternalStore stays consistent. - if (targetId !== undefined) return container.get(targetId); - if (collectionArgument) return collectionVersionRef.current; - // All-cells / ids / selector-only forms: the immutable snapshot reference - // changes on every commit and doubles as the change token. - return container.getSnapshot(); - }, [container, collectionArgument, targetId]); - - // ── Equality ── - - const isEqualCallback = useMemo(() => { - if (isEqual) return isEqual as unknown as UnknownEqual; - if (targetId === undefined && !hasSelector) { - return (a, b) => areArraysShallowEqual(a as readonly unknown[], b as readonly unknown[]); - } - if (hasSelector) return arrayAwareEqual; - return Object.is; - }, [isEqual, targetId, hasSelector]); - - // ── Selector ── - - const select = useCallback( - (): Result => { - const subscribedIds = collectionArgument ? collectionIdsRef.current : idsRef.current; - const next = computeNext, Selected>( - container, - targetId, - subscribedIds, - collectionArgument, - fallbackCacheRef.current, - arraySelectorRef.current, - cellSelectorRef.current - ); - // Reuse the cached value when equal so the returned reference stays stable - // across commits that didn't affect this subscription. - if (cachedRef.current.hasValue && isEqualCallback(cachedRef.current.value, next)) { - return cachedRef.current.value; - } - if (hasSelector && cachedRef.current.hasValue) { - warnUnstableSelector('useCells', cachedRef.current.value, next, !!isEqual); - } - // No defensive copy for the all-cells form: the container's snapshot is - // already immutable and yields a fresh reference on every commit, so it is - // safe to return as-is. - cachedRef.current = { hasValue: true, value: next }; - return next; - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [container, collectionArgument, targetId, idsKey, isEqualCallback] - ); - - return useSyncExternalStoreWithSelector( - subscribe, - getSnapshot, - getSnapshot, - select, - isEqualCallback +>(...args: CellsArguments): Selected | CellsSource { + const [argument1, argument2, argument3] = args; + const isSelectorFirst = typeof argument1 === 'function'; + // The overloads pair each target with its selector shape, which the argument + // unions hide from TypeScript. + const selector = (isSelectorFirst ? argument1 : argument2) as + | AnyCellsSelector + | undefined; + const hasSelector = typeof selector === 'function'; + // A nullish id next to a selector reads no cell; on its own it reads all cells. + let target: CellsTarget; + if (!isSelectorFirst) target = argument1 ?? (hasSelector ? null : undefined); + const isEqual = (isSelectorFirst ? argument2 : argument3) as + | SelectedEqual> + | undefined; + + const select = useCellsStateSelector( + target, + toTargetKey(target), + hasSelector ? selector : undefined ); + return useSelectGraphState('useCells', select, isEqual); } diff --git a/packages/joint-react/src/hooks/use-cells.types.ts b/packages/joint-react/src/hooks/use-cells.types.ts new file mode 100644 index 0000000000..1e9a485fb8 --- /dev/null +++ b/packages/joint-react/src/hooks/use-cells.types.ts @@ -0,0 +1,24 @@ +import { type mvc, type dia } from '@joint/core'; +import type { AnyCellRecord, CellId, Computed } from '../types/cell.types'; + +/** Selector over the resolved cells array (the array forms of `useCells`). */ +export type CellsSelector = ( + cells: ReadonlyArray> +) => Selected; + +/** + * What a cells hook is scoped to: one cell, several cells, or a JointJS + * collection. `undefined` is all cells, `null` is no cell. + */ +export type CellsTarget = CellId | null | undefined | readonly CellId[] | mvc.Collection; + +/** What a target resolves to: the cells, one cell, or nothing. */ +export type CellsSource = + | ReadonlyArray> + | Computed + | undefined; + +/** A selector of any `useCells` form; the overloads pair each target with its own shape. */ +export type AnyCellsSelector = ( + source: CellsSource +) => Selected; diff --git a/packages/joint-react/src/hooks/use-cells.utils.ts b/packages/joint-react/src/hooks/use-cells.utils.ts index c1d49c2019..05a4f4fe5d 100644 --- a/packages/joint-react/src/hooks/use-cells.utils.ts +++ b/packages/joint-react/src/hooks/use-cells.utils.ts @@ -1,129 +1,13 @@ -import { type mvc, type dia } from '@joint/core'; -import type { AnyCellRecord, CellId } from '../types/cell.types'; -import { isCollection } from '../utils/is'; - -/** Normalised arguments after dispatching by the runtime call shape. */ -export interface ParsedCellsArgs { - readonly targetId: CellId | undefined; - readonly ids: readonly CellId[] | undefined; - readonly isCollectionForm: boolean; - readonly arraySelector: ((cells: readonly Cell[]) => Selected) | undefined; - readonly cellSelector: ((cell: Cell | undefined) => Selected) | undefined; - readonly isEqual: ((a: Selected, b: Selected) => boolean) | undefined; -} - -/** Builds default parsed result with no selectors. */ -function defaultParsedArgs( - targetId: CellId | undefined, - ids?: readonly CellId[] -): ParsedCellsArgs { - return { - targetId, - ids, - isCollectionForm: false, - arraySelector: undefined, - cellSelector: undefined, - isEqual: undefined, - }; -} - -/** - * Parses the collection form: `useCells(collection)` or `useCells(collection, selector, isEqual?)`. - */ -function parseCollectionArgs( - argument2: unknown, - argument3: unknown -): ParsedCellsArgs { - return { - targetId: undefined, - ids: undefined, - isCollectionForm: true, - arraySelector: - typeof argument2 === 'function' - ? (argument2 as (cells: readonly Cell[]) => Selected) - : undefined, - cellSelector: undefined, - isEqual: - typeof argument3 === 'function' - ? (argument3 as (a: Selected, b: Selected) => boolean) - : undefined, - }; -} - -/** Parses the `useCells(selector, isEqual?)` form. */ -function parseSelectorFirstArgs( - selectorArgument: (cells: readonly Cell[]) => Selected, - isEqualArgument: unknown -): ParsedCellsArgs { - return { - targetId: undefined, - ids: undefined, - isCollectionForm: false, - arraySelector: selectorArgument, - cellSelector: undefined, - isEqual: - typeof isEqualArgument === 'function' - ? (isEqualArgument as (a: Selected, b: Selected) => boolean) - : undefined, - }; -} +import type { CellsTarget } from './use-cells.types'; /** - * Classifies the runtime arguments of {@link useCells} into a normalised shape so - * the hook body can stay flat and cheap to read. - * @param argument1 - first positional arg (id, ids, collection, or selector) - * @param argument2 - second positional arg (selector or isEqual depending on form) - * @param argument3 - third positional arg (isEqual when the form admits it) - * @returns the normalised input + * A stable stand-in for a target in a dependency list: ids passed inline are a + * new array on every render. Serialised rather than joined, so that `[1]`, + * `['1']` and the single id `'1'` stay apart. + * @param target - The target of a cells hook. + * @returns The serialised ids of an ids target, otherwise the target itself. + * @internal */ -export function parseUseCellsArgs( - argument1?: - | CellId - | null - | readonly CellId[] - | ((cells: readonly Cell[]) => Selected) - | mvc.Collection, - argument2?: - | ((cells: readonly Cell[]) => Selected) - | ((cell: Cell | undefined) => Selected) - | ((a: Selected, b: Selected) => boolean), - argument3?: (a: Selected, b: Selected) => boolean -): ParsedCellsArgs { - if (isCollection(argument1)) { - return parseCollectionArgs(argument2, argument3); - } - if (typeof argument1 === 'function') { - return parseSelectorFirstArgs(argument1, argument2); - } - - if (Array.isArray(argument1)) { - return { - targetId: undefined, - ids: argument1 as readonly CellId[], - isCollectionForm: false, - arraySelector: - typeof argument2 === 'function' - ? (argument2 as (cells: readonly Cell[]) => Selected) - : undefined, - cellSelector: undefined, - isEqual: typeof argument3 === 'function' ? argument3 : undefined, - }; - } - - // Single-cell form: an explicit id, or a nullish id treated as "no match" - // (the selector then receives `undefined`). A nullish id without a selector - // collapses to the no-arg full-array form. - const targetId: CellId | undefined = - argument1 === undefined || argument1 === null ? undefined : (argument1 as CellId); - if (typeof argument2 === 'function') { - return { - targetId, - ids: undefined, - isCollectionForm: false, - arraySelector: undefined, - cellSelector: argument2 as (cell: Cell | undefined) => Selected, - isEqual: typeof argument3 === 'function' ? argument3 : undefined, - }; - } - return defaultParsedArgs(targetId); +export function toTargetKey(target: CellsTarget): unknown { + return Array.isArray(target) ? JSON.stringify(target) : target; } diff --git a/packages/joint-react/src/hooks/use-create-portal-paper.tsx b/packages/joint-react/src/hooks/use-create-portal-paper.tsx index d14efb428f..0c6d45f06f 100644 --- a/packages/joint-react/src/hooks/use-create-portal-paper.tsx +++ b/packages/joint-react/src/hooks/use-create-portal-paper.tsx @@ -44,7 +44,6 @@ import { SVGElementItem, } from '../components/paper/render-element/paper-element-item'; import { createSelectPaperVersion } from '../selectors'; -import { useAreElementsMeasured } from './use-are-elements-measured'; import { LINK_MODEL_TYPE } from '../mvc/link-model'; import { subscribeToPaperEvents } from './use-on-paper-events'; import { useOnEvents } from './use-on-events'; @@ -209,7 +208,6 @@ export function useCreatePortalPaper( } const graphStore = useGraphStore(); - const areElementsMeasured = useAreElementsMeasured(); // The set of cell ids — stable across data-only commits (a drag does zero work // here), a new reference only on add/remove. Partition by type for portals. @@ -481,20 +479,11 @@ export function useCreatePortalPaper( - + ) : ( - + )} ); @@ -503,7 +492,6 @@ export function useCreatePortalPaper( }, [ version, HTMLRendererContainer, - areElementsMeasured, elementIds, hasRenderElement, paperStore, diff --git a/packages/joint-react/src/hooks/use-graph-state-reader.ts b/packages/joint-react/src/hooks/use-graph-state-reader.ts new file mode 100644 index 0000000000..164bd0d960 --- /dev/null +++ b/packages/joint-react/src/hooks/use-graph-state-reader.ts @@ -0,0 +1,20 @@ +import { useMemo } from 'react'; +import { useGraphStore } from './use-graph-store'; +import type { AnyCellRecord } from '../types/cell.types'; +import { createGraphStateReader, type GraphStateReader } from '../store/graph-state'; + +/** + * The reader behind {@link useGraphState}, one + * per hook instance and per graph store. + * @param hookName - The hook the application called, named in dev warnings. + * @returns The reader of the closest graph store. + * @internal + */ +export function useGraphStateReader( + hookName: string +): GraphStateReader { + const store = useGraphStore(); + // Should React drop this memo, the new reader selects afresh: correct, at the + // cost of one new reference for a selected array or object. + return useMemo(() => createGraphStateReader(store, hookName), [store, hookName]); +} diff --git a/packages/joint-react/src/hooks/use-graph-state.ts b/packages/joint-react/src/hooks/use-graph-state.ts new file mode 100644 index 0000000000..6da39ad09a --- /dev/null +++ b/packages/joint-react/src/hooks/use-graph-state.ts @@ -0,0 +1,51 @@ +import { useCallback, useEffect, useSyncExternalStore } from 'react'; +import type { AnyCellRecord, CellRecord } from '../types/cell.types'; +import type { GraphStateSelector, SelectedEqual } from '../store/graph-state'; +import { useGraphStateReader } from './use-graph-state-reader'; + +/** + * Subscribe to a value selected from the graph state: the cells and the + * sizing version. The component re-renders when the selected value changes. + * The hook subscribes to exactly what the selector reads; `useCells` is built + * on it. + * @param selector - Derives the value from the graph state. + * @param isEqual - Equality test used to skip re-renders. Defaults to a shallow, + * array-aware comparison that falls back to `Object.is` for other values. + * @returns The selected value, the same reference for as long as it is equal. + * @internal + */ +export function useGraphState( + selector: GraphStateSelector, + isEqual?: SelectedEqual +): Selected { + return useSelectGraphState('useGraphState', selector, isEqual); +} + +/** + * {@link useGraphState} for a hook built on it, which its dev warnings name. + * @param hookName - The hook the application called. + * @param selector - Derives the value from the graph state. + * @param isEqual - Equality test used to skip re-renders. + * @returns The selected value, the same reference for as long as it is equal. + * @internal + */ +export function useSelectGraphState( + hookName: string, + selector: GraphStateSelector, + isEqual?: SelectedEqual +): Selected { + const reader = useGraphStateReader(hookName); + const getSnapshot = useCallback( + () => reader.read(selector, isEqual), + [reader, selector, isEqual] + ); + const selected = useSyncExternalStore(reader.subscribe, getSnapshot, getSnapshot); + // A committed selector may read other parts of the state than the one before + // it. The subscription follows it here, never while rendering: a render that + // React throws away must leave it where the committed one needs it. Declared + // after the store subscription, so on mount it runs once that is in place. + useEffect(() => { + reader.sync(selector, isEqual); + }, [reader, selector, isEqual]); + return selected; +} diff --git a/packages/joint-react/src/hooks/use-measure-element.tsx b/packages/joint-react/src/hooks/use-measure-element.tsx index c8481d1e37..63fc07ab09 100644 --- a/packages/joint-react/src/hooks/use-measure-element.tsx +++ b/packages/joint-react/src/hooks/use-measure-element.tsx @@ -1,7 +1,7 @@ import { useContext, useLayoutEffect, type RefObject } from 'react'; import { CellIdContext } from '../context'; import { useGraphStore } from './use-graph-store'; -import type { TransformElementLayout } from '../store/create-elements-size-observer'; +import type { TransformElementLayout } from '../store/measurement'; import { usePaper } from './use-paper'; import type { ElementSize } from '../types/cell.types'; import { useCell } from './use-cell'; @@ -142,7 +142,8 @@ export function useMeasureElement( options: MeasureElementOptions = EMPTY_OBJECT ): Required { const { transform } = options; - const { graph, setMeasuredNode } = useGraphStore(); + const graphStore = useGraphStore(); + const { graph, measurement } = graphStore; const { paper } = usePaper(); const id = useContext(CellIdContext); if (id === undefined) { @@ -179,15 +180,18 @@ export function useMeasureElement( paper.requestViewUpdate(view, paper.FLAG_MEASURE, view.UPDATE_PRIORITY); } - const clean = setMeasuredNode({ id, node: nodeRef.current, transform }); - return () => { - // No class cleanup here: views aren't recycled today, so the view - // is gone with the cell. - clean(); - }; + // No class cleanup: views aren't recycled today, so the view is gone with the cell. + return measurement.observe({ id, node: nodeRef.current, transform }); // transform is not a dependency because it doesn't change // eslint-disable-next-line react-hooks/exhaustive-deps - }, [nodeRef, graph, id, paper, setMeasuredNode]); + }, [nodeRef, graph, id, paper, measurement]); + + // The paper may route the links before React commits a measured size (the + // application may flush the views from a measurement callback), so let + // them resolve again against the committed content. + useLayoutEffect(() => { + if (paper) graphStore.clearViewForElementAndLinks({ cellId: id, paper }); + }, [graphStore, id, paper, size]); return size; } diff --git a/packages/joint-react/src/hooks/use-on-elements-measured.ts b/packages/joint-react/src/hooks/use-on-elements-measured.ts index 0715133e97..bb12f00745 100644 --- a/packages/joint-react/src/hooks/use-on-elements-measured.ts +++ b/packages/joint-react/src/hooks/use-on-elements-measured.ts @@ -1,9 +1,10 @@ -import { useLayoutEffect, useRef } from 'react'; +import { useLayoutEffect } from 'react'; import type { dia } from '@joint/core'; import { usePaperStore, useResolvePaperId } from './use-paper'; import type { PaperTarget } from '../types'; import { useGraphStore } from './use-graph-store'; import { useLatestRef } from './use-latest-ref'; +import { rethrowLater } from '../utils/scheduler'; /** * Payload passed to the {@link useOnElementsMeasured} callback after a @@ -33,9 +34,8 @@ export type OnElementsMeasured = (params: ElementsMeasuredParams) => void; /** * Calls a callback once element sizes are known, so a layout can run on them. * - * Delivers one event per settled change: the first pass (at least one element - * has a size), and each later addition or re-measurement, once no element is - * still waiting. An added element waits until the paper has rendered it; if + * Delivers one event per settled change: the first pass, and each later + * addition, removal or re-measurement, once no element is still waiting. An added element waits until the paper has rendered it; if * its content measures itself ({@link useMeasureElement}, {@link HTMLHost}), * until that measurement arrives. A batch mixing plain and measured elements * is one event, delivered when the last one is measured. An element the paper @@ -46,8 +46,9 @@ export type OnElementsMeasured = (params: ElementsMeasuredParams) => void; * every size change. * * The callback receives {@link ElementsMeasuredParams}; `isInitial` is `true` - * for the first event after the hook mounts and again for the first event - * after a graph reset (`resetCells()`), which replaces the diagram. + * for the first event after the hook mounts, and again for the first event + * after a graph reset (`resetCells()`), which replaces the diagram, or after + * the last element was removed. * @title On the current paper * @param callback - Called each time element sizes are measured. * @group Hooks @@ -98,34 +99,35 @@ export function useOnElementsMeasured( const isContextForm = typeof paperTargetOrCallback === 'function'; const paperTarget = isContextForm ? undefined : (paperTargetOrCallback as PaperTarget); - const callback = isContextForm ? (paperTargetOrCallback as OnElementsMeasured) : (callbackArgument as OnElementsMeasured); + const callback = isContextForm + ? (paperTargetOrCallback as OnElementsMeasured) + : (callbackArgument as OnElementsMeasured); const paperId = useResolvePaperId(paperTarget); const paperStore = usePaperStore(paperId); const callbackRef = useLatestRef(callback); - const graphStore = useGraphStore(); - const { measureState, graph } = graphStore; - // The `measureGeneration` this hook last reported; a generation it has not - // seen yet (mount, graph reset) makes the next event `isInitial`. - const reportedGenerationRef = useRef(-1); + const { measurement, graph } = useGraphStore(); useLayoutEffect(() => { if (!paperStore) return; const { paper } = paperStore; - // A new paper (or graph store) starts its own measurement history, so its - // first pass reports `isInitial: true` again — e.g. after a dev-server hot - // reload re-created the store, `transformToFitContent()` callers re-fit. - reportedGenerationRef.current = -1; + // A new paper (or graph store) starts its own history, and so does a graph + // reset (the state goes back to `0`): the next event is the initial one. + let previousState = 0; function handleChanges() { - // A graph reset clears `measureState`; the reset diagram's own first - // pass is reported once it settles. - if (measureState.get() === 0) return; - const { measureGeneration } = graphStore; - const isInitial = reportedGenerationRef.current !== measureGeneration; - reportedGenerationRef.current = measureGeneration; - callbackRef.current({ isInitial, paper, graph }); + const state = measurement.getSizingVersion(); + const isInitial = previousState === 0; + previousState = state; + if (state === 0) return; + // This runs inside the store's notification: an error must not keep the + // other subscribers of this change from being notified. + try { + callbackRef.current({ isInitial, paper, graph }); + } catch (error) { + rethrowLater(error); + } // The user callback may have moved cells via cell.position()/cell.size(). // PaperView runs in async mode, so those updates would be queued for the // next rAF — producing a one-frame flash where the element is visible at its @@ -135,12 +137,7 @@ export function useOnElementsMeasured( } // Flush any measurement that happened before subscription (e.g. initial // data sync ran before this paperStore was available). - if (measureState.get() > 0) { - handleChanges(); - } - const unsubscribe = measureState.subscribe(handleChanges); - return () => { - unsubscribe(); - }; - }, [paperStore, graphStore, measureState, graph, callbackRef]); + handleChanges(); + return measurement.subscribe(handleChanges); + }, [paperStore, measurement, graph, callbackRef]); } diff --git a/packages/joint-react/src/index.ts b/packages/joint-react/src/index.ts index 9ae4148f34..368aabb4f4 100644 --- a/packages/joint-react/src/index.ts +++ b/packages/joint-react/src/index.ts @@ -12,7 +12,7 @@ export { GraphProvider } from './components/graph/graph-provider'; /** @group Types */ export type { GraphProviderProps } from './components/graph/graph-provider'; /** @group Types */ -export type { AutoSizeOrigin } from './store/graph-store'; +export type { AutoSizeOrigin } from './store/measurement'; /** @group Types */ export type { IncrementalCellsChange } from './store/graph-projection'; @@ -132,10 +132,7 @@ export { useMeasureElement } from './hooks/use-measure-element'; /** @group Types */ export type { MeasureElementOptions } from './hooks/use-measure-element'; /** @group Types */ -export type { - TransformElementLayout, - TransformElementLayoutParams, -} from './store/create-elements-size-observer'; +export type { TransformElementLayout, TransformElementLayoutParams } from './store/measurement'; /** * useOnElementsMeasured() diff --git a/packages/joint-react/src/internal.ts b/packages/joint-react/src/internal.ts index 99fb9fb8d5..c6cbbe17a3 100644 --- a/packages/joint-react/src/internal.ts +++ b/packages/joint-react/src/internal.ts @@ -27,7 +27,8 @@ export type { FeaturesContext as FeaturesContextType } from './context'; export { FeaturesProvider } from './components'; // Store Classes -export { GraphStore, DEFAULT_CELL_NAMESPACE, AUTO_SIZE_OPTION } from './store/graph-store'; +export { GraphStore, DEFAULT_CELL_NAMESPACE } from './store/graph-store'; +export { AUTO_SIZE_OPTION } from './store/measurement'; export type { GraphStoreInternalSnapshot, PaperStoreState } from './store/graph-store'; export type { GraphStoreOptions } from './store/graph-store'; export { PaperStore } from './store/paper-store'; diff --git a/packages/joint-react/src/utils/__tests__/selector-utils.test.ts b/packages/joint-react/src/selectors/__tests__/selector-utils.test.ts similarity index 100% rename from packages/joint-react/src/utils/__tests__/selector-utils.test.ts rename to packages/joint-react/src/selectors/__tests__/selector-utils.test.ts diff --git a/packages/joint-react/src/selectors/__tests__/index.test.ts b/packages/joint-react/src/selectors/__tests__/store-selectors.test.ts similarity index 93% rename from packages/joint-react/src/selectors/__tests__/index.test.ts rename to packages/joint-react/src/selectors/__tests__/store-selectors.test.ts index 8180ffd7b5..7ed1161cf9 100644 --- a/packages/joint-react/src/selectors/__tests__/index.test.ts +++ b/packages/joint-react/src/selectors/__tests__/store-selectors.test.ts @@ -2,10 +2,10 @@ import { selectResetVersion, createSelectPaperVersion, selectGraphFeaturesVersion, -} from '../index'; +} from '../store-selectors'; import type { GraphStoreInternalSnapshot } from '../../store/graph-store'; -describe('selectors/index', () => { +describe('store-selectors', () => { const snapshot: GraphStoreInternalSnapshot = { papers: { 'paper-1': { version: 7 } as GraphStoreInternalSnapshot['papers'][string], diff --git a/packages/joint-react/src/selectors/index.ts b/packages/joint-react/src/selectors/index.ts index a141f876cb..9907166368 100644 --- a/packages/joint-react/src/selectors/index.ts +++ b/packages/joint-react/src/selectors/index.ts @@ -1,25 +1,5 @@ -import type { GraphStoreInternalSnapshot } from '../store/graph-store'; - -// ── Public cell selectors ─────────────────────────────────────────────────── - +// Cell selectors (`selectCell*`, `selectElement*`), passed to `useCell` / `useCells`. export * from './cell-selectors'; -// ── Internal selectors ────────────────────────────────────────────────────── - -export const selectResetVersion = (state: GraphStoreInternalSnapshot): number => state.resetVersion; - -/** - * Creates a selector for the version of a specific paper. - * @param id - The paper ID to select the version for. - */ -export function createSelectPaperVersion(id: string) { - return (snapshot: GraphStoreInternalSnapshot) => snapshot.papers[id]?.version; -} - -/** - * Selects the graph features version from the internal snapshot. - * Used to trigger re-renders when graph-level features change. - * @param state - */ -export const selectGraphFeaturesVersion = (state: GraphStoreInternalSnapshot): number => - state.graphFeaturesVersion; +// Internal selectors of the graph store snapshot. +export * from './store-selectors'; diff --git a/packages/joint-react/src/utils/selector-utils.ts b/packages/joint-react/src/selectors/selector-utils.ts similarity index 92% rename from packages/joint-react/src/utils/selector-utils.ts rename to packages/joint-react/src/selectors/selector-utils.ts index 82c20771c7..a78c635a13 100644 --- a/packages/joint-react/src/utils/selector-utils.ts +++ b/packages/joint-react/src/selectors/selector-utils.ts @@ -1,6 +1,5 @@ import type { ElementPosition, ElementSize } from '../types/cell.types'; - export const isStrictEqual = Object.is; export const identitySelector = (item: T) => item; @@ -10,10 +9,7 @@ export const identitySelector = (item: T) => item; * @param a * @param b */ -export function isShallowEqual( - a: object | undefined, - b: object | undefined -): boolean { +export function isShallowEqual(a: object | undefined, b: object | undefined): boolean { if (a === b) return true; if (!a || !b) return false; const objectA = a as Record; @@ -55,7 +51,7 @@ export function isPositionEqual(a?: ElementPosition, b?: ElementPosition): boole * @param b - second array * @returns true when both arrays have the same length and the same element refs */ -export function areArraysShallowEqual(a: readonly T[], b: readonly T[]): boolean { +function areArraysShallowEqual(a: readonly T[], b: readonly T[]): boolean { if (a === b) return true; if (a.length !== b.length) return false; for (const [index, value] of a.entries()) { diff --git a/packages/joint-react/src/selectors/store-selectors.ts b/packages/joint-react/src/selectors/store-selectors.ts new file mode 100644 index 0000000000..a852b40ef0 --- /dev/null +++ b/packages/joint-react/src/selectors/store-selectors.ts @@ -0,0 +1,19 @@ +import type { GraphStoreInternalSnapshot } from '../store/graph-store'; + +export const selectResetVersion = (state: GraphStoreInternalSnapshot): number => state.resetVersion; + +/** + * Creates a selector for the version of a specific paper. + * @param id - The paper ID to select the version for. + */ +export function createSelectPaperVersion(id: string) { + return (snapshot: GraphStoreInternalSnapshot) => snapshot.papers[id]?.version; +} + +/** + * Selects the graph features version from the internal snapshot. + * Used to trigger re-renders when graph-level features change. + * @param state + */ +export const selectGraphFeaturesVersion = (state: GraphStoreInternalSnapshot): number => + state.graphFeaturesVersion; diff --git a/packages/joint-react/src/state/data-mapping/cell-record-merge.ts b/packages/joint-react/src/state/data-mapping/cell-record-merge.ts index b8ccc98961..675857926c 100644 --- a/packages/joint-react/src/state/data-mapping/cell-record-merge.ts +++ b/packages/joint-react/src/state/data-mapping/cell-record-merge.ts @@ -1,6 +1,6 @@ import type { dia } from '@joint/core'; import type { ElementJSONInit, LinkJSONInit } from '../../types/cell.types'; -import { isShallowEqual, isPositionEqual, isSizeEqual } from '../../utils/selector-utils'; +import { isShallowEqual, isPositionEqual, isSizeEqual } from '../../selectors/selector-utils'; import { mapAttributesToElement } from './element-mapper'; import { mapAttributesToLink } from './link-mapper'; diff --git a/packages/joint-react/src/store/__tests__/clear-view.test.ts b/packages/joint-react/src/store/__tests__/clear-view.test.ts index af789664e9..05c5b1ebc3 100644 --- a/packages/joint-react/src/store/__tests__/clear-view.test.ts +++ b/packages/joint-react/src/store/__tests__/clear-view.test.ts @@ -1,11 +1,5 @@ import { dia } from '@joint/core'; -import { - clearConnectedLinkViews, - executeClearViewForCell, - mergeClearViewValidators, - shouldClearLink, - type ClearViewCacheEntry, -} from '../clear-view'; +import { clearConnectedLinkViews, shouldClearLink } from '../clear-view'; import { DEFAULT_CELL_NAMESPACE } from '../graph-store'; function createGraph(): dia.Graph { @@ -26,55 +20,6 @@ function createMockLinkView(): MockLinkView { }; } -describe('mergeClearViewValidators', () => { - it('returns incoming when there is no existing entry', () => { - const incoming: ClearViewCacheEntry = { onValidateLink: () => true }; - const result = mergeClearViewValidators(undefined, incoming); - expect(result).toBe(incoming); - }); - - it('takes precedence with no validator (clear all)', () => { - const existing: ClearViewCacheEntry = { onValidateLink: () => true }; - const incoming: ClearViewCacheEntry = {}; - const result = mergeClearViewValidators(existing, incoming); - expect(result.onValidateLink).toBeUndefined(); - }); - - it('creates a union when both have validators', () => { - const existingValidator = jest.fn().mockReturnValue(false); - const newValidator = jest.fn().mockReturnValue(true); - const result = mergeClearViewValidators( - { onValidateLink: existingValidator }, - { onValidateLink: newValidator } - ); - expect(typeof result.onValidateLink).toBe('function'); - const link = {} as dia.Link; - expect(result.onValidateLink!(link)).toBe(true); - expect(existingValidator).toHaveBeenCalledWith(link); - expect(newValidator).toHaveBeenCalledWith(link); - }); - - it('union short-circuits when existing returns true', () => { - const existingValidator = jest.fn().mockReturnValue(true); - const newValidator = jest.fn().mockReturnValue(false); - const result = mergeClearViewValidators( - { onValidateLink: existingValidator }, - { onValidateLink: newValidator } - ); - const link = {} as dia.Link; - expect(result.onValidateLink!(link)).toBe(true); - expect(newValidator).not.toHaveBeenCalled(); - }); - - it('keeps the existing "clear all" semantics when existing has no validator', () => { - const existing: ClearViewCacheEntry = {}; - const incoming: ClearViewCacheEntry = { onValidateLink: () => true }; - const result = mergeClearViewValidators(existing, incoming); - expect(result).toBe(existing); - expect(result.onValidateLink).toBeUndefined(); - }); -}); - function makeLink(sourceId: string, targetId: string): dia.Link { return new dia.Link({ type: 'standard.Link', @@ -84,7 +29,6 @@ function makeLink(sourceId: string, targetId: string): dia.Link { } describe('shouldClearLink', () => { - it('returns true when source matches', () => { const link = makeLink('a', 'b'); expect(shouldClearLink(link, 'a')).toBe(true); @@ -179,7 +123,9 @@ describe('clearConnectedLinkViews', () => { graph.addCell(link); // Override `findView` to simulate the no-view-registered case. - link.findView = jest.fn(() => undefined as unknown as dia.LinkView) as unknown as typeof link.findView; + link.findView = jest.fn( + () => undefined as unknown as dia.LinkView + ) as unknown as typeof link.findView; const paper = { findViewByModel: jest.fn(), @@ -226,74 +172,3 @@ describe('clearConnectedLinkViews', () => { expect(mockLinkView.requestConnectionUpdate).toHaveBeenCalledWith({ async: true }); }); }); - -describe('executeClearViewForCell', () => { - let graph: dia.Graph; - - beforeEach(() => { - graph = createGraph(); - }); - - it('iterates papers and skips ones without a paper instance', () => { - graph.addCell({ - id: 'a', - type: 'element', - position: { x: 0, y: 0 }, - size: { width: 10, height: 10 }, - }); - const papers = [{ paper: undefined }]; - expect(() => executeClearViewForCell(papers, graph, 'a')).not.toThrow(); - }); - - it('skips papers where the cell view is not present', () => { - graph.addCell({ - id: 'a', - type: 'element', - position: { x: 0, y: 0 }, - size: { width: 10, height: 10 }, - }); - - const getCellView = jest.fn(); - const paper = { getCellView } as unknown as dia.Paper; - - executeClearViewForCell([{ paper }], graph, 'a'); - expect(getCellView).toHaveBeenCalledWith('a'); - }); - - it('cleans the cell view nodes cache and clears connected link views', () => { - graph.addCell({ - id: 'a', - type: 'element', - position: { x: 0, y: 0 }, - size: { width: 10, height: 10 }, - }); - graph.addCell({ - id: 'b', - type: 'element', - position: { x: 50, y: 0 }, - size: { width: 10, height: 10 }, - }); - const link = new dia.Link({ - id: 'l1', - type: 'standard.Link', - source: { id: 'a' }, - target: { id: 'b' }, - }); - graph.addCell(link); - - const cleanNodesCache = jest.fn(); - const elementView = { cleanNodesCache } as unknown as dia.ElementView; - - const mockLinkView = createMockLinkView(); - link.findView = jest.fn().mockReturnValue(mockLinkView) as unknown as typeof link.findView; - - const paper = { - getCellView: jest.fn().mockReturnValue(elementView), - } as unknown as dia.Paper; - - executeClearViewForCell([{ paper }], graph, 'a'); - expect(cleanNodesCache).toHaveBeenCalledTimes(1); - expect(mockLinkView.requestConnectionUpdate).toHaveBeenCalled(); - }); -}); - diff --git a/packages/joint-react/src/store/__tests__/create-elements-size-observer.test.ts b/packages/joint-react/src/store/__tests__/create-elements-size-observer.test.ts deleted file mode 100644 index fe8622e5b2..0000000000 --- a/packages/joint-react/src/store/__tests__/create-elements-size-observer.test.ts +++ /dev/null @@ -1,608 +0,0 @@ -/* eslint-disable prefer-destructuring */ - -/* eslint-disable @typescript-eslint/no-require-imports */ -import type { dia } from '@joint/core'; -import type { CellId } from '../../types/cell.types'; -import type { GraphStoreObserver } from '../create-elements-size-observer'; - -// Mock ResizeObserver for testing - -let mockResizeObserverInstances: MockResizeObserver[] = []; - -class MockResizeObserver { - private callback: ResizeObserverCallback; - private observedElements = new Map(); - - constructor(callback: ResizeObserverCallback) { - this.callback = callback; - mockResizeObserverInstances.push(this); - } - - observe(target: Element) { - // Simulate an entry with initial size - const entry = this.createEntry(target, 100, 50); - this.observedElements.set(target, entry); - } - - unobserve(target: Element) { - this.observedElements.delete(target); - } - - disconnect() { - this.observedElements.clear(); - } - - // Test helper to simulate resize - triggerResize(target: Element, width: number, height: number) { - const entry = this.createEntry(target, width, height); - this.observedElements.set(target, entry); - this.callback([entry], this as unknown as ResizeObserver); - } - - // Test helper to trigger callback for all observed elements - triggerAllCallbacks() { - const entries = [...this.observedElements.values()]; - if (entries.length > 0) { - this.callback(entries, this as unknown as ResizeObserver); - } - } - - isObserving(target: Element): boolean { - return this.observedElements.has(target); - } - - get observedCount(): number { - return this.observedElements.size; - } - - private createEntry(target: Element, width: number, height: number): ResizeObserverEntry { - return { - target, - contentRect: { - width, - height, - top: 0, - left: 0, - bottom: height, - right: width, - x: 0, - y: 0, - toJSON: () => ({}), - }, - borderBoxSize: [{ inlineSize: width, blockSize: height }], - contentBoxSize: [{ inlineSize: width, blockSize: height }], - devicePixelContentBoxSize: [{ inlineSize: width, blockSize: height }], - } as ResizeObserverEntry; - } - - static getLastInstance(): MockResizeObserver | undefined { - return mockResizeObserverInstances.at(-1); - } - - static clearInstances() { - mockResizeObserverInstances = []; - } -} - -// Replace global ResizeObserver with mock -globalThis.ResizeObserver = MockResizeObserver as unknown as typeof ResizeObserver; - -describe('createElementsSizeObserver', () => { - let observer: GraphStoreObserver; - let mockOnBatchUpdate: jest.Mock; - let mockGetCellTransform: jest.Mock; - let mockGetElements: jest.Mock; - let mockElements: Map; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let createElementsSizeObserver: any; - - beforeEach(() => { - MockResizeObserver.clearInstances(); - - // Reset modules and reimport to ensure the mock is used - jest.resetModules(); - // Re-assign the mock after reset to ensure it's used - globalThis.ResizeObserver = MockResizeObserver as unknown as typeof ResizeObserver; - - // jest.resetModules() requires synchronous re-import; using `require` is the only - // option here. ESM `import()` is async and does not bypass module cache the same way. - // eslint-disable-next-line @typescript-eslint/no-var-requires - createElementsSizeObserver = require('../create-elements-size-observer').createElementsSizeObserver; - - mockElements = new Map([ - ['element-1', { size: { width: 1, height: 1 }, position: { x: 0, y: 0 } }], - ['element-2', { size: { width: 1, height: 1 }, position: { x: 0, y: 0 } }], - ]); - - mockOnBatchUpdate = jest.fn(); - mockGetCellTransform = jest.fn((id: CellId) => ({ - width: 1, - height: 1, - x: 0, - y: 0, - angle: 0, - element: { id } as dia.Element, - })); - mockGetElements = jest.fn(() => mockElements); - - observer = createElementsSizeObserver({ - onBatchUpdate: mockOnBatchUpdate, - getCellTransform: mockGetCellTransform, - getElements: mockGetElements, - }); - }); - - afterEach(() => { - observer.clean(); - }); - - describe('add', () => { - it('should register element with ResizeObserver', () => { - const element = document.createElement('div'); - - observer.add({ id: 'element-1', node: element }); - - expect(observer.has('element-1')).toBe(true); - }); - - it('should return cleanup function that unregisters element', () => { - const element = document.createElement('div'); - - const cleanup = observer.add({ id: 'element-1', node: element }); - expect(observer.has('element-1')).toBe(true); - - cleanup(); - expect(observer.has('element-1')).toBe(false); - }); - - it('should handle multiple elements', () => { - const element1 = document.createElement('div'); - const element2 = document.createElement('div'); - - observer.add({ id: 'element-1', node: element1 }); - observer.add({ id: 'element-2', node: element2 }); - - expect(observer.has('element-1')).toBe(true); - expect(observer.has('element-2')).toBe(true); - }); - - it('should process ResizeObserver callback when element is added', () => { - const element = document.createElement('div'); - - observer.add({ id: 'element-1', node: element }); - - // Trigger resize via ResizeObserver (simulates browser behavior) - const resizeObserver = MockResizeObserver.getLastInstance(); - expect(resizeObserver).toBeDefined(); - resizeObserver?.triggerResize(element, 100, 50); - - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(1); - - const updateCall = mockOnBatchUpdate.mock.calls[0][0]; - expect(updateCall['element-1']).toBeDefined(); - expect(updateCall['element-1'].width).toBe(100); - expect(updateCall['element-1'].height).toBe(50); - }); - - it('should handle multiple elements with ResizeObserver', () => { - const element1 = document.createElement('div'); - const element2 = document.createElement('div'); - - observer.add({ id: 'element-1', node: element1 }); - observer.add({ id: 'element-2', node: element2 }); - - // Trigger resize for both elements - const resizeObserver = MockResizeObserver.getLastInstance(); - resizeObserver?.triggerResize(element1, 100, 50); - resizeObserver?.triggerResize(element2, 200, 100); - - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(2); - }); - }); - - describe('ResizeObserver callback', () => { - it('should process size changes from ResizeObserver', () => { - const element = document.createElement('div'); - - observer.add({ id: 'element-1', node: element }); - - const resizeObserver = MockResizeObserver.getLastInstance(); - expect(resizeObserver).toBeDefined(); - - // Trigger initial resize - resizeObserver?.triggerResize(element, 100, 50); - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(1); - - // Trigger resize to a different size - resizeObserver?.triggerResize(element, 200, 100); - - // Should be called again for the resize - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(2); - }); - - it('should not update if size has not changed significantly', () => { - const element = document.createElement('div'); - - observer.add({ id: 'element-1', node: element }); - - const resizeObserver = MockResizeObserver.getLastInstance(); - - // Trigger initial resize - resizeObserver?.triggerResize(element, 100, 50); - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(1); - mockOnBatchUpdate.mockClear(); - - // Trigger resize with same size (within epsilon of 0.5) - resizeObserver?.triggerResize(element, 100.1, 50.1); - - // Should not trigger update because change is within epsilon - expect(mockOnBatchUpdate).not.toHaveBeenCalled(); - }); - - it('should use transform function when provided', () => { - const element = document.createElement('div'); - const transform = jest.fn(({ width, height }) => ({ - width: width + 20, - height: height + 20, - })); - - observer.add({ id: 'element-1', node: element, transform }); - - const resizeObserver = MockResizeObserver.getLastInstance(); - resizeObserver?.triggerResize(element, 100, 50); - - expect(transform).toHaveBeenCalled(); - - const updateCall = mockOnBatchUpdate.mock.calls[0][0]; - expect(updateCall['element-1'].width).toBe(120); // 100 + 20 - expect(updateCall['element-1'].height).toBe(70); // 50 + 20 - }); - }); - - describe('stack behavior', () => { - it('should allow multiple registrations for the same cell ID', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - observer.add({ id: 'element-1', node: nodeA }); - observer.add({ id: 'element-1', node: nodeB }); - - expect(observer.has('element-1')).toBe(true); - }); - - it('should only observe the active (latest) node via ResizeObserver', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - observer.add({ id: 'element-1', node: nodeA }); - observer.add({ id: 'element-1', node: nodeB }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // nodeA should be unobserved (deactivated when nodeB was added) - expect(resizeObserver.isObserving(nodeA)).toBe(false); - // nodeB should be observed (active) - expect(resizeObserver.isObserving(nodeB)).toBe(true); - }); - - it('should process resize only from the active node', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - observer.add({ id: 'element-1', node: nodeA }); - observer.add({ id: 'element-1', node: nodeB }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // Trigger resize on the active node (nodeB) - resizeObserver.triggerResize(nodeB, 200, 100); - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(1); - - const updateCall = mockOnBatchUpdate.mock.calls[0][0]; - expect(updateCall['element-1'].width).toBe(200); - expect(updateCall['element-1'].height).toBe(100); - }); - - it('should fall back to previous node when active node is removed', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - observer.add({ id: 'element-1', node: nodeA }); - const cleanupB = observer.add({ id: 'element-1', node: nodeB }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // Remove the active node (nodeB) - cleanupB(); - - // nodeA should be re-activated - expect(resizeObserver.isObserving(nodeA)).toBe(true); - expect(resizeObserver.isObserving(nodeB)).toBe(false); - expect(observer.has('element-1')).toBe(true); - - // Trigger resize on the re-activated nodeA - resizeObserver.triggerResize(nodeA, 150, 75); - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(1); - - const updateCall = mockOnBatchUpdate.mock.calls[0][0]; - expect(updateCall['element-1'].width).toBe(150); - expect(updateCall['element-1'].height).toBe(75); - }); - - it('should remove non-active node without affecting active observation', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - const cleanupA = observer.add({ id: 'element-1', node: nodeA }); - observer.add({ id: 'element-1', node: nodeB }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // Remove the non-active node (nodeA) - cleanupA(); - - // nodeB should still be active - expect(resizeObserver.isObserving(nodeB)).toBe(true); - expect(observer.has('element-1')).toBe(true); - - // Trigger resize on nodeB — should still work - resizeObserver.triggerResize(nodeB, 300, 150); - expect(mockOnBatchUpdate).toHaveBeenCalledTimes(1); - }); - - it('should handle removing all entries one by one (3-deep stack)', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - const nodeC = document.createElement('div'); - - observer.add({ id: 'element-1', node: nodeA }); - observer.add({ id: 'element-1', node: nodeB }); - const cleanupC = observer.add({ id: 'element-1', node: nodeC }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // Remove C (active) → B becomes active - cleanupC(); - expect(resizeObserver.isObserving(nodeC)).toBe(false); - expect(resizeObserver.isObserving(nodeB)).toBe(true); - expect(observer.has('element-1')).toBe(true); - }); - - it('should fully remove cell ID after all stack entries are cleaned up', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - const cleanupA = observer.add({ id: 'element-1', node: nodeA }); - const cleanupB = observer.add({ id: 'element-1', node: nodeB }); - - cleanupB(); - expect(observer.has('element-1')).toBe(true); - - cleanupA(); - expect(observer.has('element-1')).toBe(false); - }); - - it('should use the active node transform function, not a previous one', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - const transformA = jest.fn(({ width, height }) => ({ - width: width + 10, - height: height + 10, - })); - const transformB = jest.fn(({ width, height }) => ({ - width: width + 50, - height: height + 50, - })); - - observer.add({ id: 'element-1', node: nodeA, transform: transformA }); - observer.add({ id: 'element-1', node: nodeB, transform: transformB }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - resizeObserver.triggerResize(nodeB, 100, 50); - - expect(transformA).not.toHaveBeenCalled(); - expect(transformB).toHaveBeenCalled(); - - const updateCall = mockOnBatchUpdate.mock.calls[0][0]; - expect(updateCall['element-1'].width).toBe(150); // 100 + 50 - expect(updateCall['element-1'].height).toBe(100); // 50 + 50 - }); - - it('should use the previous transform after active node is removed', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - - const transformA = jest.fn(({ width, height }) => ({ - width: width + 10, - height: height + 10, - })); - const transformB = jest.fn(({ width, height }) => ({ - width: width + 50, - height: height + 50, - })); - - observer.add({ id: 'element-1', node: nodeA, transform: transformA }); - const cleanupB = observer.add({ id: 'element-1', node: nodeB, transform: transformB }); - - // Remove B, A becomes active with its own transform - cleanupB(); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - resizeObserver.triggerResize(nodeA, 100, 50); - - expect(transformA).toHaveBeenCalled(); - - const updateCall = mockOnBatchUpdate.mock.calls[0][0]; - expect(updateCall['element-1'].width).toBe(110); // 100 + 10 - expect(updateCall['element-1'].height).toBe(60); // 50 + 10 - }); - - it('cleanup should be idempotent', () => { - const nodeA = document.createElement('div'); - - const cleanup = observer.add({ id: 'element-1', node: nodeA }); - - cleanup(); - expect(observer.has('element-1')).toBe(false); - - // Second cleanup call should not throw or cause side effects - cleanup(); - expect(observer.has('element-1')).toBe(false); - }); - - it('should handle interleaved add/remove across different cell IDs', () => { - const node1A = document.createElement('div'); - const node1B = document.createElement('div'); - const node2A = document.createElement('div'); - - const cleanup1A = observer.add({ id: 'element-1', node: node1A }); - observer.add({ id: 'element-2', node: node2A }); - observer.add({ id: 'element-1', node: node1B }); - - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // element-1 active is node1B, element-2 active is node2A - expect(resizeObserver.isObserving(node1B)).toBe(true); - expect(resizeObserver.isObserving(node2A)).toBe(true); - expect(resizeObserver.isObserving(node1A)).toBe(false); - - // Remove non-active node from element-1 - cleanup1A(); - expect(resizeObserver.isObserving(node1B)).toBe(true); - expect(observer.has('element-1')).toBe(true); - expect(observer.has('element-2')).toBe(true); - }); - }); - - describe('clean', () => { - it('should remove all observed elements', () => { - const element1 = document.createElement('div'); - const element2 = document.createElement('div'); - - observer.add({ id: 'element-1', node: element1 }); - observer.add({ id: 'element-2', node: element2 }); - - expect(observer.has('element-1')).toBe(true); - expect(observer.has('element-2')).toBe(true); - - observer.clean(); - - expect(observer.has('element-1')).toBe(false); - expect(observer.has('element-2')).toBe(false); - }); - - it('should clean all stacks including multi-entry stacks', () => { - const nodeA = document.createElement('div'); - const nodeB = document.createElement('div'); - const nodeC = document.createElement('div'); - - observer.add({ id: 'element-1', node: nodeA }); - observer.add({ id: 'element-1', node: nodeB }); - observer.add({ id: 'element-2', node: nodeC }); - - observer.clean(); - - expect(observer.has('element-1')).toBe(false); - expect(observer.has('element-2')).toBe(false); - }); - }); - - describe('has', () => { - it('should return true for registered elements', () => { - const element = document.createElement('div'); - observer.add({ id: 'element-1', node: element }); - - expect(observer.has('element-1')).toBe(true); - }); - - it('should return false for unregistered elements', () => { - expect(observer.has('non-existent')).toBe(false); - }); - }); - - describe('processSizeChange branches', () => { - it('skips when elements map does not contain the cell id', () => { - const element = document.createElement('div'); - mockElements.clear(); // ← getElements() returns an empty map - observer.add({ id: 'element-1', node: element }); - const resizeObserver = MockResizeObserver.getLastInstance()!; - resizeObserver.triggerResize(element, 100, 50); - expect(mockOnBatchUpdate).not.toHaveBeenCalled(); - }); - - it('skips when cell transform already matches the measured size (within epsilon)', () => { - const element = document.createElement('div'); - mockGetCellTransform.mockImplementation((id: CellId) => ({ - width: 100, - height: 50, - x: 0, - y: 0, - angle: 0, - element: { id } as dia.Element, - })); - observer.add({ id: 'element-1', node: element }); - const resizeObserver = MockResizeObserver.getLastInstance()!; - resizeObserver.triggerResize(element, 100, 50); - expect(mockOnBatchUpdate).not.toHaveBeenCalled(); - }); - }); - - describe('ResizeObserver entry edge cases', () => { - it('skips entries with no borderBoxSize', () => { - const element = document.createElement('div'); - observer.add({ id: 'element-1', node: element }); - const resizeObserver = MockResizeObserver.getLastInstance()!; - - // Construct an entry with empty borderBoxSize manually - const callback = (resizeObserver as unknown as { callback: ResizeObserverCallback }) - .callback; - callback( - [ - { - target: element, - contentRect: {} as DOMRectReadOnly, - borderBoxSize: [], - contentBoxSize: [], - devicePixelContentBoxSize: [], - } as ResizeObserverEntry, - ], - resizeObserver as unknown as ResizeObserver - ); - - expect(mockOnBatchUpdate).not.toHaveBeenCalled(); - }); - - it('skips entries with zero size (e.g. display:none)', () => { - const element = document.createElement('div'); - observer.add({ id: 'element-1', node: element }); - const resizeObserver = MockResizeObserver.getLastInstance()!; - - resizeObserver.triggerResize(element, 0, 0); - expect(mockOnBatchUpdate).not.toHaveBeenCalled(); - }); - - it('skips entries whose target is not currently observed', () => { - const element = document.createElement('div'); - observer.add({ id: 'element-1', node: element }); - const resizeObserver = MockResizeObserver.getLastInstance()!; - const otherNode = document.createElement('div'); - - const callback = (resizeObserver as unknown as { callback: ResizeObserverCallback }) - .callback; - callback( - [ - { - target: otherNode, - contentRect: {} as DOMRectReadOnly, - borderBoxSize: [{ inlineSize: 100, blockSize: 50 }], - contentBoxSize: [{ inlineSize: 100, blockSize: 50 }], - devicePixelContentBoxSize: [{ inlineSize: 100, blockSize: 50 }], - } as ResizeObserverEntry, - ], - resizeObserver as unknown as ResizeObserver - ); - expect(mockOnBatchUpdate).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/packages/joint-react/src/store/__tests__/graph-changes.test.ts b/packages/joint-react/src/store/__tests__/graph-changes.test.ts index f431b34dc6..5aefa16e49 100644 --- a/packages/joint-react/src/store/__tests__/graph-changes.test.ts +++ b/packages/joint-react/src/store/__tests__/graph-changes.test.ts @@ -19,22 +19,6 @@ function setup() { return { graph, onChanges, controller }; } -function setupWithSize() { - const graph = createGraph(); - const onChanges = jest.fn(); - const onElementsSizeChange = jest.fn(); - const onElementRemove = jest.fn(); - const onReset = jest.fn(); - const controller = graphChanges({ - graph, - onChanges, - onElementsSizeChange, - onElementRemove, - onReset, - }); - return { graph, onChanges, onElementsSizeChange, onElementRemove, onReset, controller }; -} - function addElement(graph: dia.Graph, id: string, x = 10, y = 20, width = 100, height = 50) { graph.addCell({ id, @@ -385,121 +369,6 @@ describe('graphChanges', () => { }); }); - describe('onElementsSizeChange', () => { - it('fires for each element when resetCells seeds cells with sizes', () => { - const { graph, onElementsSizeChange } = setupWithSize(); - graph.resetCells([ - { - id: 'a', - type: 'element', - position: { x: 0, y: 0 }, - size: { width: 100, height: 50 }, - }, - { - id: 'b', - type: 'element', - position: { x: 200, y: 0 }, - size: { width: 80, height: 40 }, - }, - { - id: 'l1', - type: 'standard.Link', - source: { id: 'a' }, - target: { id: 'b' }, - }, - ]); - - const elementCalls = onElementsSizeChange.mock.calls.filter( - ([id]) => id === 'a' || id === 'b' - ); - expect(elementCalls).toHaveLength(2); - expect(elementCalls).toEqual( - expect.arrayContaining([ - ['a', { width: 100, height: 50 }], - ['b', { width: 80, height: 40 }], - ]) - ); - }); - - it('fires onReset before the seed cells\' size notifications', () => { - const { graph, onElementsSizeChange, onReset } = setupWithSize(); - onReset.mockImplementation(() => { - expect(onElementsSizeChange).not.toHaveBeenCalled(); - }); - graph.resetCells([ - { id: 'a', type: 'element', position: { x: 0, y: 0 }, size: { width: 100, height: 50 } }, - ]); - - expect(onReset).toHaveBeenCalledTimes(1); - expect(onElementsSizeChange).toHaveBeenCalledTimes(1); - }); - - it('fires onElementRemove for elements only', () => { - const { graph, onElementRemove } = setupWithSize(); - addElement(graph, 'a'); - addElement(graph, 'b'); - addLink(graph, 'l1', 'a', 'b'); - - graph.getCell('l1').remove(); - expect(onElementRemove).not.toHaveBeenCalled(); - - graph.getCell('a').remove(); - expect(onElementRemove).toHaveBeenCalledWith('a'); - }); - - it('forwards the change:size options of a measurement write', () => { - const { graph, onElementsSizeChange } = setupWithSize(); - graph.resetCells([ - { - id: 'a', - type: 'element', - position: { x: 0, y: 0 }, - size: { width: 0, height: 0 }, - }, - ]); - onElementsSizeChange.mockClear(); - - const cell = graph.getCell('a') as dia.Element; - cell.set('size', { width: 120, height: 60 }, { autoSize: true } as object); - - // The `change:size` options are forwarded so the store can tell a - // measurement write from an application resize (#3514). - expect(onElementsSizeChange).toHaveBeenCalledWith( - 'a', - { width: 120, height: 60 }, - expect.objectContaining({ autoSize: true }) - ); - }); - - it('does not fire for links on reset', () => { - const { graph, onElementsSizeChange } = setupWithSize(); - graph.resetCells([ - { - id: 'a', - type: 'element', - position: { x: 0, y: 0 }, - size: { width: 100, height: 50 }, - }, - { - id: 'b', - type: 'element', - position: { x: 200, y: 0 }, - size: { width: 80, height: 40 }, - }, - { - id: 'l1', - type: 'standard.Link', - source: { id: 'a' }, - target: { id: 'b' }, - }, - ]); - - for (const [id] of onElementsSizeChange.mock.calls) { - expect(id).not.toBe('l1'); - } - }); - }); - describe('destroy', () => { it('stops listening to graph events', () => { const { graph, onChanges, controller } = setup(); diff --git a/packages/joint-react/src/store/__tests__/graph-state.test.ts b/packages/joint-react/src/store/__tests__/graph-state.test.ts new file mode 100644 index 0000000000..dbc9d0d405 --- /dev/null +++ b/packages/joint-react/src/store/__tests__/graph-state.test.ts @@ -0,0 +1,385 @@ +import { dia, mvc } from '@joint/core'; +import { GraphStore } from '../graph-store'; +import { createGraphStateReader, type GraphState } from '../graph-state'; +import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model'; +import { AUTO_SIZE_OPTION } from '../measurement'; +import type { CellRecord } from '../../types/cell.types'; + +const element = (id: string, x = 0): CellRecord => + ({ + id, + type: ELEMENT_MODEL_TYPE, + position: { x, y: 0 }, + size: { width: 10, height: 10 }, + }) as CellRecord; + +const flush = () => new Promise((resolve) => queueMicrotask(resolve)); +const selectX = (id: string) => (state: GraphState) => state.getCell(id)?.position?.x; +const selectCount = (state: GraphState) => state.cells.length; +const selectState = (state: GraphState) => state; +const selectMeasured = (state: GraphState) => state.sizingVersion; +const selectVersionAndWidth = (state: GraphState) => + `${state.sizingVersion > 0}:${state.getCell('a')?.size?.width}`; +const selectIds = (state: GraphState) => state.cells.map((cell) => cell.id); +const selectPicked = (state: GraphState) => + state.getCells(['b', 'missing', 'a']).map((cell) => cell.id); +const selectAAndLate = (state: GraphState) => state.getCells(['a', 'late']); +/** Reads the cells only once the diagram is measured. */ +const selectCountOnceMeasured = (state: GraphState) => + state.sizingVersion > 0 ? state.cells.length : -1; +const selectAndThrow = (state: GraphState) => { + state.getCell('b'); + throw new Error('selector failed'); +}; +const selectMemberIds = (collection: mvc.Collection) => (state: GraphState) => + state.getCells(collection).map((cell) => cell.id); +const selectMembers = (collection: mvc.Collection) => (state: GraphState) => + state.getCells(collection); +const isSameParity = (a: number, b: number) => a % 2 === b % 2; +const MANY_IDS = Array.from({ length: 50 }, (_, index) => `n${index}`); +const MANY_ELEMENTS = MANY_IDS.map((id) => element(id)); +const createStore = () => new GraphStore({ initialCells: [element('a'), element('b', 50)] }); +/** Changes the graph and drains the store's microtask commit. */ +const commit = async (run: () => void) => { + run(); + await flush(); +}; + +describe('createGraphStateReader', () => { + let store: ReturnType; + const getElement = (id: string) => store.graph.getCell(id) as dia.Element; + + beforeEach(() => { + store = createStore(); + }); + afterEach(() => store.destroy(false)); + + describe('read', () => { + it('selects from all cells, one cell, several cells and the sizing version', async () => { + await flush(); + const reader = createGraphStateReader(store); + + expect(reader.read(selectCount)).toBe(2); + expect(reader.read(selectX('b'))).toBe(50); + expect(reader.read(selectX('missing'))).toBeUndefined(); + expect(reader.read(selectPicked)).toEqual(['b', 'a']); + expect(reader.read(selectMeasured)).toBe(store.measurement.getSizingVersion()); + }); + + it('does not run the selector again while what it read is unchanged', async () => { + const reader = createGraphStateReader(store); + const selector = jest.fn(selectX('a')); + + reader.read(selector); + reader.read(selector); + // Another cell changed: this selector read only `a`. + await commit(() => getElement('b').position(70, 0)); + reader.read(selector); + + expect(selector).toHaveBeenCalledTimes(1); + }); + + // Regression: the render-to-subscribe gap. Staleness is tested against the + // store, so a change is seen without any subscription delivering it. + it('sees a change that happened while nothing was subscribed', async () => { + const reader = createGraphStateReader(store); + const selector = selectX('a'); + expect(reader.read(selector)).toBe(0); + + await commit(() => getElement('a').position(30, 0)); + + expect(reader.read(selector)).toBe(30); + }); + + it('runs a new selector function even when the store did not change', () => { + const reader = createGraphStateReader(store); + + expect(reader.read(selectX('a'))).toBe(0); + expect(reader.read(selectX('b'))).toBe(50); + }); + + it('returns the previous reference while the new value is equal', async () => { + const reader = createGraphStateReader(store); + const before = reader.read(selectIds); + await commit(() => getElement('a').position(30, 0)); + + expect(reader.read(selectIds)).toBe(before); + }); + + it('uses a custom equality', async () => { + const reader = createGraphStateReader(store); + expect(reader.read(selectCount, isSameParity)).toBe(2); + await commit(() => store.graph.addCells([element('c'), element('d')])); + expect(reader.read(selectCount, isSameParity)).toBe(2); + await commit(() => store.graph.addCell(element('e'))); + expect(reader.read(selectCount, isSameParity)).toBe(5); + }); + + it('recovers after a selector that throws', () => { + const reader = createGraphStateReader(store); + expect(reader.read(selectX('a'))).toBe(0); + + expect(() => reader.read(selectAndThrow)).toThrow('selector failed'); + expect(reader.read(selectX('a'))).toBe(0); + }); + }); + + // The reader follows what a selector reads while it runs. A state kept and + // read later would silently never update, so it says so. + it('warns, and follows nothing, when the state is read outside of a selector', async () => { + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const reader = createGraphStateReader(store); + const listener = jest.fn(); + const leaked = reader.read(selectState); + reader.subscribe(listener); + + expect(leaked.getCell('a')?.id).toBe('a'); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('outside of its selector')); + + await commit(() => getElement('a').position(30, 0)); + expect(listener).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + describe('subscribe', () => { + it('subscribes only to what the selector read', async () => { + const { cells } = store.graphProjection; + const subscribeToAll = jest.spyOn(cells, 'subscribe'); + const subscribeById = jest.spyOn(cells, 'subscribeById'); + const subscribeToMeasurement = jest.spyOn(store.measurement, 'subscribe'); + const reader = createGraphStateReader(store); + const listener = jest.fn(); + + reader.read(selectX('a')); + const unsubscribe = reader.subscribe(listener); + + expect(subscribeById).toHaveBeenCalledTimes(1); + expect(subscribeById).toHaveBeenCalledWith('a', expect.any(Function)); + expect(subscribeToAll).not.toHaveBeenCalled(); + expect(subscribeToMeasurement).not.toHaveBeenCalled(); + + await commit(() => getElement('b').position(70, 0)); + expect(listener).not.toHaveBeenCalled(); + await commit(() => getElement('a').position(30, 0)); + expect(listener).toHaveBeenCalledTimes(1); + + unsubscribe(); + await commit(() => getElement('a').position(40, 0)); + expect(listener).toHaveBeenCalledTimes(1); + }); + + // Regression: the sizing version was announced in the same scheduler round + // as the measured write, ahead of the projection commit of that write, so + // a selector reading cells next to the version saw the pre-measure sizes. + it('announces a new sizing version once the measured sizes are in the cells', async () => { + await flush(); + const reader = createGraphStateReader(store); + const seen: string[] = []; + reader.sync(selectVersionAndWidth); + reader.subscribe(() => seen.push(reader.sync(selectVersionAndWidth))); + + await commit(() => { + store.graph.startBatch('auto-size'); + getElement('a').set('size', { width: 99, height: 10 }, { [AUTO_SIZE_OPTION]: true }); + store.graph.stopBatch('auto-size'); + }); + await flush(); + + expect(seen).not.toContain('true:10'); + expect(seen.at(-1)).toBe('true:99'); + }); + + it('is not notified of a cell change when the selector reads the sizing version', async () => { + await flush(); + const reader = createGraphStateReader(store); + const listener = jest.fn(); + reader.read(selectMeasured); + reader.subscribe(listener); + + await commit(() => getElement('a').position(30, 0)); + expect(listener).not.toHaveBeenCalled(); + + await commit(() => store.graph.addCell(element('c'))); + expect(listener).toHaveBeenCalled(); + }); + + // Regression: the render-to-subscribe gap, with the subscription in place + // by the time of the next read. Nothing notified it of the change. + it('sees a change that happened between the read and the subscription', async () => { + const reader = createGraphStateReader(store); + const selector = selectX('a'); + expect(reader.read(selector)).toBe(0); + + await commit(() => getElement('a').position(30, 0)); + reader.subscribe(jest.fn()); + + expect(reader.read(selector)).toBe(30); + }); + + it('moves the subscription when the selector reads something else', async () => { + const reader = createGraphStateReader(store); + const listener = jest.fn(); + reader.read(selectX('a')); + reader.subscribe(listener); + + reader.sync(selectX('b')); + + await commit(() => getElement('a').position(30, 0)); + expect(listener).not.toHaveBeenCalled(); + await commit(() => getElement('b').position(70, 0)); + expect(listener).toHaveBeenCalledTimes(1); + }); + + // Regression: a read alone moved the subscription. React reads while + // rendering, and a render it throws away (a transition, a suspended tree) + // left the committed selector without its notifications. + it('keeps the subscription where it is on a read that does not sync', async () => { + const reader = createGraphStateReader(store); + const listener = jest.fn(); + const committed = selectX('a'); + reader.read(committed); + reader.subscribe(() => { + listener(); + reader.read(committed); + }); + + // What a discarded render does: reads with another selector, and no more. + reader.read(selectX('b')); + + await commit(() => getElement('a').position(30, 0)); + expect(listener).toHaveBeenCalledTimes(1); + expect(reader.read(committed)).toBe(30); + }); + + it('follows a selector that reads conditionally', async () => { + const emptyStore = new GraphStore({}); + const reader = createGraphStateReader(emptyStore); + const listener = jest.fn(); + const selector = selectCountOnceMeasured; + expect(reader.read(selector)).toBe(-1); + reader.subscribe(() => { + listener(); + reader.read(selector); + }); + + // Measured now: the listener's read picks up the cells as a second source. + emptyStore.graph.addCells([element('a'), element('b')]); + await flush(); + expect(reader.read(selector)).toBe(2); + listener.mockClear(); + + emptyStore.graph.getCell('a').set('position', { x: 30, y: 0 }); + await flush(); + expect(listener).toHaveBeenCalled(); + emptyStore.destroy(false); + }); + + // Regression: every notification scanned all the watched cells, so one + // commit moving k of N watched cells cost k scans of N. + it('reads each watched cell a constant number of times when all of them change at once', async () => { + const ids = MANY_IDS; + store.graph.addCells(MANY_ELEMENTS); + await flush(); + const reader = createGraphStateReader(store); + const selector = jest.fn((state: GraphState) => state.getCells(ids)); + reader.read(selector); + // What a subscribed hook does: read again on every notification. + reader.subscribe(() => reader.read(selector)); + selector.mockClear(); + const getCell = jest.spyOn(store.graphProjection.cells, 'get'); + + await commit(() => { + store.graph.startBatch('move'); + for (const id of ids) getElement(id).position(5, 5); + store.graph.stopBatch('move'); + }); + + expect(selector).toHaveBeenCalledTimes(1); + // One lookup per notification and one per id in the selector run, not ids². + expect(getCell.mock.calls.length).toBeLessThanOrEqual(ids.length * 3); + }); + + it('is notified when a missing cell it asked for is added', async () => { + const reader = createGraphStateReader(store); + const listener = jest.fn(); + const selector = selectAAndLate; + expect(reader.read(selector)).toHaveLength(1); + reader.subscribe(listener); + + await commit(() => store.graph.addCell(element('late'))); + + expect(listener).toHaveBeenCalled(); + expect(reader.read(selector)).toHaveLength(2); + }); + }); + + describe('collections', () => { + it('reads the members of a collection, in its order, and follows membership', () => { + const collection = new mvc.Collection([getElement('b')]); + const reader = createGraphStateReader(store); + const listener = jest.fn(); + const selector = selectMemberIds(collection); + expect(reader.read(selector)).toEqual(['b']); + reader.subscribe(listener); + + collection.add(getElement('a')); + expect(listener).toHaveBeenCalledTimes(1); + expect(reader.read(selector)).toEqual(['b', 'a']); + + collection.remove(getElement('b')); + expect(reader.read(selector)).toEqual(['a']); + + collection.reset([]); + expect(reader.read(selector)).toEqual([]); + }); + + // Regression: membership was tracked by a counter the subscription bumped, + // so a change before the subscription existed was missed. + it('sees a membership change that happened while nothing was subscribed', () => { + const collection = new mvc.Collection(); + const reader = createGraphStateReader(store); + const selector = selectMemberIds(collection); + expect(reader.read(selector)).toEqual([]); + + collection.add(getElement('a')); + + expect(reader.read(selector)).toEqual(['a']); + }); + + it('is notified once for several cells added together', () => { + const collection = new mvc.Collection(); + const reader = createGraphStateReader(store); + const listener = jest.fn(); + reader.read(selectMemberIds(collection)); + reader.subscribe(listener); + + collection.add([getElement('a'), getElement('b')]); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('reads a member the graph does not hold from the collection, as one stable record', () => { + const detached = new dia.Element({ id: 'clone', type: ELEMENT_MODEL_TYPE }); + const collection = new mvc.Collection([detached]); + const reader = createGraphStateReader(store); + const first = reader.read(selectMembers(collection)); + const second = reader.read(selectMembers(collection)); + + expect(first.map((cell) => (cell as CellRecord).id)).toEqual(['clone']); + expect(second[0]).toBe(first[0]); + }); + + it('stops listening to the collection on unsubscribe', () => { + const collection = new mvc.Collection(); + const reader = createGraphStateReader(store); + const listener = jest.fn(); + reader.read(selectMemberIds(collection)); + const unsubscribe = reader.subscribe(listener); + + unsubscribe(); + collection.add(getElement('a')); + + expect(listener).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/joint-react/src/store/__tests__/graph-store-features.test.ts b/packages/joint-react/src/store/__tests__/graph-store-features.test.ts index 5557aea252..a93988adb8 100644 --- a/packages/joint-react/src/store/__tests__/graph-store-features.test.ts +++ b/packages/joint-react/src/store/__tests__/graph-store-features.test.ts @@ -343,7 +343,7 @@ describe('GraphStore observer wiring', () => { }); const node = document.createElement('div'); - const cleanup = store.setMeasuredNode({ id: 'a', node }); + const cleanup = store.measurement.observe({ id: 'a', node }); expect(typeof cleanup).toBe('function'); cleanup(); store.destroy(false); @@ -499,7 +499,7 @@ describe('GraphStore size observer integration', () => { }); const node = document.createElement('div'); - store.setMeasuredNode({ id: 'a', node }); + store.measurement.observe({ id: 'a', node }); const observer = MockResizeObserver.instances.at(-1)!; observer.triggerResize(node, 100, 50); @@ -522,7 +522,7 @@ describe('GraphStore size observer integration', () => { }); const node = document.createElement('div'); - store.setMeasuredNode({ + store.measurement.observe({ id: 'a', node, transform: ({ width, height }) => ({ width, height, x: 9, y: 11 }), @@ -562,7 +562,7 @@ describe('GraphStore size observer integration', () => { }); const node = document.createElement('div'); - store.setMeasuredNode({ id: 'a', node }); + store.measurement.observe({ id: 'a', node }); const observer = MockResizeObserver.instances.at(-1)!; observer.triggerResize(node, 50, 25); @@ -581,7 +581,7 @@ describe('GraphStore size observer integration', () => { size: { width: 1, height: 1 }, }); const node = document.createElement('div'); - store.setMeasuredNode({ id: 'ghost', node }); + store.measurement.observe({ id: 'ghost', node }); // Remove the cell — the projection no longer contains it, so // processSizeChange bails out before reaching getCellTransform. diff --git a/packages/joint-react/src/store/__tests__/graph-store.test.ts b/packages/joint-react/src/store/__tests__/graph-store.test.ts index 142f41d60b..6d4dcfdc04 100644 --- a/packages/joint-react/src/store/__tests__/graph-store.test.ts +++ b/packages/joint-react/src/store/__tests__/graph-store.test.ts @@ -8,6 +8,9 @@ import type { CellRecord } from '../../types/cell.types'; const createGraph = () => new dia.Graph({}, { cellNamespace: DEFAULT_CELL_NAMESPACE }); const flush = () => new Promise((resolve) => queueMicrotask(resolve)); +const sizingVersion = (store: { + readonly measurement: { readonly getSizingVersion: () => number }; +}) => store.measurement.getSizingVersion(); describe('GraphStore', () => { describe('constructor', () => { @@ -61,7 +64,7 @@ describe('GraphStore', () => { store.destroy(true); }); - it('bumps measureState after initialCells seed (so useOnElementsMeasured can fire isInitial)', async () => { + it('bumps the measurement version after the initialCells seed', async () => { const initialCells: readonly CellRecord[] = [ { id: 'a', @@ -71,14 +74,14 @@ describe('GraphStore', () => { } as CellRecord, ]; const store = new GraphStore({ initialCells }); - // simpleScheduler defers the measureState bump to a microtask. + // simpleScheduler defers the bump to a microtask. await flush(); - expect(store.measureState.get()).toBeGreaterThan(0); + expect(sizingVersion(store)).toBeGreaterThan(0); store.destroy(false); }); - it('does not bump measureState when initialCells contain only links or zero-sized elements', async () => { - const initialCells: readonly CellRecord[] = [ + it('counts a zero-sized element as measured (zero is a legal size), but not a diagram of links only', async () => { + const zeroSized: readonly CellRecord[] = [ { id: 'zero', type: ELEMENT_MODEL_TYPE, @@ -86,35 +89,49 @@ describe('GraphStore', () => { size: { width: 0, height: 0 }, } as CellRecord, ]; - const store = new GraphStore({ initialCells }); + const store = new GraphStore({ initialCells: zeroSized }); await flush(); - expect(store.measureState.get()).toBe(0); + expect(sizingVersion(store)).toBe(1); store.destroy(false); + + const linksOnly = new GraphStore({ + initialCells: [ + { + id: 'l', + type: LINK_MODEL_TYPE, + source: { x: 0, y: 0 }, + target: { x: 9, y: 9 }, + } as CellRecord, + ], + }); + await flush(); + expect(sizingVersion(linksOnly)).toBe(0); + linksOnly.destroy(false); }); - it('bumps measureState once an unsized element is measured to the size it already has', async () => { + it('does not change the measured state for an application resize or an equal measurement', async () => { const initialCells: readonly CellRecord[] = [ { id: 'a', type: ELEMENT_MODEL_TYPE, position: { x: 0, y: 0 } } as CellRecord, ]; const store = new GraphStore({ initialCells }); await flush(); - // Nothing has a size yet, so the seed pass is not delivered. - expect(store.measureState.get()).toBe(0); - // The application pre-sizes the waiting element: not a measurement. + // No paper renders it, so the seed settles on its own. + expect(sizingVersion(store)).toBe(1); + // The application sizes the element: not a measurement. (store.graph.getCell('a') as dia.Element).resize(120, 40); await flush(); - expect(store.measureState.get()).toBe(0); + expect(sizingVersion(store)).toBe(1); // The observer measures the same size, so it writes nothing to the graph. const node = document.createElement('div'); - store.setMeasuredNode({ id: 'a', node }); + store.measurement.observe({ id: 'a', node }); const [callback] = (globalThis.ResizeObserver as jest.Mock).mock.calls.at(-1) as [ ResizeObserverCallback, ]; const entry = { target: node, borderBoxSize: [{ inlineSize: 120, blockSize: 40 }] }; callback([entry as unknown as ResizeObserverEntry], {} as ResizeObserver); await flush(); - expect(store.measureState.get()).toBe(1); + expect(sizingVersion(store)).toBe(1); store.destroy(false); }); @@ -221,7 +238,9 @@ describe('GraphStore', () => { it('fires with added/changed/removed summary', async () => { const snaps: Array> = []; const store = new GraphStore({}); - store.setOnIncrementalCellsChange((c) => snaps.push(snapshot(c as Parameters[0]))); + store.setOnIncrementalCellsChange((c) => + snaps.push(snapshot(c as Parameters[0])) + ); store.graph.addCell({ id: 'a', type: ELEMENT_MODEL_TYPE, @@ -239,7 +258,9 @@ describe('GraphStore', () => { it('reports links alongside elements in the unified pipeline', async () => { const snaps: Array> = []; const store = new GraphStore({}); - store.setOnIncrementalCellsChange((c) => snaps.push(snapshot(c as Parameters[0]))); + store.setOnIncrementalCellsChange((c) => + snaps.push(snapshot(c as Parameters[0])) + ); store.graph.addCells([ { id: 'a', @@ -294,6 +315,29 @@ describe('GraphStore', () => { }); }); + describe('removing a paper', () => { + // Regression: only papers hold an outstanding element back, and nothing + // re-checked the delivery when the last one went away, so the sizing + // version stayed `0` until the next graph change. + it('settles the elements that waited for the last paper', async () => { + const store = new GraphStore({}); + const { remove } = store.addPaper('p1', { paperOptions: {} }); + store.graph.addCell({ + id: 'a', + type: ELEMENT_MODEL_TYPE, + position: { x: 0, y: 0 }, + size: { width: 10, height: 10 }, + }); + await flush(); + expect(sizingVersion(store)).toBe(0); + + remove(); + await flush(); + expect(sizingVersion(store)).toBe(1); + store.destroy(false); + }); + }); + describe('destroy', () => { it('clears features, papers, and internal state', () => { const store = new GraphStore({}); @@ -317,7 +361,7 @@ describe('GraphStore', () => { const measure = (store: GraphStore, id: string) => { const node = document.createElement('div'); document.body.append(node); - store.setMeasuredNode({ id, node }); + store.measurement.observe({ id, node }); return () => node.remove(); }; diff --git a/packages/joint-react/src/store/__tests__/measurement.test.ts b/packages/joint-react/src/store/__tests__/measurement.test.ts new file mode 100644 index 0000000000..2d7b574b6a --- /dev/null +++ b/packages/joint-react/src/store/__tests__/measurement.test.ts @@ -0,0 +1,709 @@ +import { dia } from '@joint/core'; +import { createMeasurement, AUTO_SIZE_OPTION, type AutoSizeOrigin } from '../measurement'; +import { DEFAULT_CELL_NAMESPACE } from '../graph-store'; +import type { PaperStore } from '../paper-store'; + +/** A ResizeObserver the test drives: `report()` is what the browser would deliver. */ +class TestResizeObserver { + static readonly instances: TestResizeObserver[] = []; + readonly observed = new Set(); + isDisconnected = false; + private readonly callback: ResizeObserverCallback; + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + TestResizeObserver.instances.push(this); + } + observe(target: Element) { + this.observed.add(target); + } + unobserve(target: Element) { + this.observed.delete(target); + } + disconnect() { + this.isDisconnected = true; + this.observed.clear(); + } + report(target: Element, width: number, height: number, hasBox = true) { + const borderBoxSize = hasBox ? [{ inlineSize: width, blockSize: height }] : []; + const entry = { target, borderBoxSize } as unknown as ResizeObserverEntry; + this.callback([entry], this as unknown as ResizeObserver); + } +} + +const flush = () => new Promise((resolve) => queueMicrotask(resolve)); +const lastObserver = () => TestResizeObserver.instances.at(-1)!; +const NO_PAPERS: ReadonlyMap = new Map(); +const newNode = () => document.createElement('div'); + +type Transform = Parameters['observe']>[0]['transform']; + +/** + * A graph with its measurement. `add` puts a 1x1 element at (10, 20) unless + * sized otherwise; `writes` counts the batches the measurement writes in, one + * per ResizeObserver callback that changed something. + */ +function setup(autoSizeOrigin: AutoSizeOrigin = 'top-left') { + const graph = new dia.Graph({}, { cellNamespace: DEFAULT_CELL_NAMESPACE }); + const measurement = createMeasurement({ graph, autoSizeOrigin, paperStores: NO_PAPERS }); + /** The sizing version right now. */ + const version = () => measurement.getSizingVersion(); + const nodesById = new Map(); + const writes = jest.fn(); + graph.on('batch:stop', ({ batchName }: { batchName?: string }) => { + if (batchName === 'auto-size') writes(); + }); + const add = (id: string, width = 1, height = 1) => { + graph.addCell({ id, type: 'element', position: { x: 10, y: 20 }, size: { width, height } }); + return graph.getCell(id) as dia.Element; + }; + const observe = (id: string, node: HTMLElement = newNode(), transform?: Transform) => { + nodesById.set(id, [...(nodesById.get(id) ?? []), node]); + return measurement.observe({ id, node, transform }); + }; + /** The old observer's `has(id)`: the element has a registered node, so one of them is observed. */ + const has = (id: string) => + (nodesById.get(id) ?? []).some((node) => + TestResizeObserver.instances.some((instance) => instance.observed.has(node)) + ); + const size = (id: string) => (graph.getCell(id) as dia.Element).size(); + return { graph, measurement, version, add, observe, has, size, writes }; +} + +beforeEach(() => { + TestResizeObserver.instances.length = 0; + globalThis.ResizeObserver = TestResizeObserver as unknown as typeof ResizeObserver; +}); + +describe('measurement — observe', () => { + it('does not create a ResizeObserver until a node is observed', () => { + const { add, observe } = setup(); + add('element-1'); + expect(TestResizeObserver.instances).toHaveLength(0); + + observe('element-1'); + observe('element-1'); + expect(TestResizeObserver.instances).toHaveLength(1); + }); + + it('registers the element with the ResizeObserver', () => { + const { add, observe, has } = setup(); + add('element-1'); + + observe('element-1'); + + expect(has('element-1')).toBe(true); + expect(has('non-existent')).toBe(false); + }); + + it('returns a cleanup that unregisters the element', () => { + const { add, observe, has } = setup(); + add('element-1'); + + const cleanup = observe('element-1'); + expect(has('element-1')).toBe(true); + + cleanup(); + expect(has('element-1')).toBe(false); + }); + + it('handles multiple elements', () => { + const { add, observe, has } = setup(); + add('element-1'); + add('element-2'); + + observe('element-1'); + observe('element-2'); + + expect(has('element-1')).toBe(true); + expect(has('element-2')).toBe(true); + }); + + it('writes the reported size to the element', () => { + const { add, observe, size, writes } = setup(); + add('element-1'); + const node = newNode(); + observe('element-1', node); + + lastObserver().report(node, 100, 50); + + expect(writes).toHaveBeenCalledTimes(1); + expect(size('element-1')).toEqual({ width: 100, height: 50 }); + }); + + it('writes each of several elements in its own batch', () => { + const { add, observe, size, writes } = setup(); + add('element-1'); + add('element-2'); + const first = newNode(); + const second = newNode(); + observe('element-1', first); + observe('element-2', second); + + lastObserver().report(first, 100, 50); + lastObserver().report(second, 200, 100); + + expect(writes).toHaveBeenCalledTimes(2); + expect(size('element-1')).toEqual({ width: 100, height: 50 }); + expect(size('element-2')).toEqual({ width: 200, height: 100 }); + }); +}); + +describe('measurement — ResizeObserver callback', () => { + it('writes every size change', () => { + const { add, observe, writes } = setup(); + add('element-1'); + const node = newNode(); + observe('element-1', node); + + lastObserver().report(node, 100, 50); + expect(writes).toHaveBeenCalledTimes(1); + + lastObserver().report(node, 200, 100); + expect(writes).toHaveBeenCalledTimes(2); + }); + + it('does not write when the size has not changed significantly', () => { + const { add, observe, writes } = setup(); + add('element-1'); + const node = newNode(); + observe('element-1', node); + lastObserver().report(node, 100, 50); + writes.mockClear(); + + // Within the 0.5 tolerance. + lastObserver().report(node, 100.1, 50.1); + + expect(writes).not.toHaveBeenCalled(); + }); + + it('tags the write as a measurement and rounds to two decimals', () => { + const { graph, add, observe, size } = setup(); + const element = add('element-1'); + const node = newNode(); + observe('element-1', node); + const onSize = jest.fn(); + graph.on('change:size', onSize); + + lastObserver().report(node, 100.004, 50); + + expect(size('element-1')).toEqual({ width: 100, height: 50 }); + expect(element.position()).toEqual({ x: 10, y: 20 }); + expect(onSize.mock.calls[0][2]).toEqual(expect.objectContaining({ [AUTO_SIZE_OPTION]: true })); + }); + + it('uses the transform when provided', () => { + const { add, observe, size } = setup(); + const element = add('element-1'); + const node = newNode(); + const transform = jest.fn(({ width, height }) => ({ width: width + 20, height: height + 20 })); + observe('element-1', node, transform); + + lastObserver().report(node, 100, 50); + + expect(transform).toHaveBeenCalledWith({ + x: 10, + y: 20, + angle: 0, + model: element, + width: 100, + height: 50, + id: 'element-1', + }); + expect(size('element-1')).toEqual({ width: 120, height: 70 }); + }); + + it('writes the position a transform returns', () => { + const { add, observe } = setup(); + const element = add('element-1'); + const node = newNode(); + observe('element-1', node, ({ width, height }) => ({ width, height, x: 9, y: 11 })); + + lastObserver().report(node, 80, 40); + + expect(element.position()).toEqual({ x: 9, y: 11 }); + }); + + it('keeps the center fixed with the center origin', () => { + const { add, observe } = setup('center'); + const element = add('element-1', 20, 20); + const node = newNode(); + observe('element-1', node); + + lastObserver().report(node, 40, 60); + + expect(element.position()).toEqual({ x: 0, y: 0 }); + }); + + it('skips an element that is not in the graph', () => { + const { observe, writes } = setup(); + const node = newNode(); + observe('element-1', node); + + expect(() => lastObserver().report(node, 100, 50)).not.toThrow(); + expect(writes).not.toHaveBeenCalled(); + }); + + it('skips an element removed from the graph after it registered', () => { + const { add, observe, writes } = setup(); + const element = add('element-1'); + const node = newNode(); + observe('element-1', node); + element.remove(); + + expect(() => lastObserver().report(node, 100, 50)).not.toThrow(); + expect(writes).not.toHaveBeenCalled(); + }); + + it('skips when the model already has the measured size (within the tolerance)', () => { + const { add, observe, writes } = setup(); + add('element-1', 100, 50); + const node = newNode(); + observe('element-1', node); + + lastObserver().report(node, 100, 50); + + expect(writes).not.toHaveBeenCalled(); + }); + + it('skips a report equal to the last one, even if the application resized meanwhile', () => { + const { add, observe, size, writes } = setup(); + const element = add('element-1'); + const node = newNode(); + observe('element-1', node); + lastObserver().report(node, 100, 50); + writes.mockClear(); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + element.resize(300, 300); + lastObserver().report(node, 100, 50); + + expect(writes).not.toHaveBeenCalled(); + expect(size('element-1')).toEqual({ width: 300, height: 300 }); + warn.mockRestore(); + }); + + it('skips entries with no borderBoxSize', () => { + const { add, observe, writes } = setup(); + add('element-1'); + const node = newNode(); + observe('element-1', node); + + lastObserver().report(node, 100, 50, false); + + expect(writes).not.toHaveBeenCalled(); + }); + + it('skips entries with zero size (e.g. display:none)', () => { + const { add, observe, writes } = setup(); + add('element-1'); + const node = newNode(); + observe('element-1', node); + + lastObserver().report(node, 0, 0); + + expect(writes).not.toHaveBeenCalled(); + }); + + it('skips entries whose target is not currently observed', () => { + const { add, observe, writes } = setup(); + add('element-1'); + observe('element-1'); + + lastObserver().report(newNode(), 100, 50); + + expect(writes).not.toHaveBeenCalled(); + }); +}); + +describe('measurement — several nodes for one element', () => { + it('allows multiple registrations for the same element', () => { + const { add, observe, has } = setup(); + add('element-1'); + + observe('element-1'); + observe('element-1'); + + expect(has('element-1')).toBe(true); + }); + + it('observes only the latest node', () => { + const { add, observe } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + + observe('element-1', nodeA); + observe('element-1', nodeB); + + expect(lastObserver().observed.has(nodeA)).toBe(false); + expect(lastObserver().observed.has(nodeB)).toBe(true); + }); + + it('writes only what the latest node reports', () => { + const { add, observe, size, writes } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + observe('element-1', nodeA); + observe('element-1', nodeB); + + lastObserver().report(nodeA, 77, 77); + expect(writes).not.toHaveBeenCalled(); + + lastObserver().report(nodeB, 200, 100); + expect(writes).toHaveBeenCalledTimes(1); + expect(size('element-1')).toEqual({ width: 200, height: 100 }); + }); + + it('falls back to the previous node when the latest one is removed', () => { + const { add, observe, has, size, writes } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + observe('element-1', nodeA); + const cleanupB = observe('element-1', nodeB); + + cleanupB(); + + expect(lastObserver().observed.has(nodeA)).toBe(true); + expect(lastObserver().observed.has(nodeB)).toBe(false); + expect(has('element-1')).toBe(true); + + lastObserver().report(nodeA, 150, 75); + expect(writes).toHaveBeenCalledTimes(1); + expect(size('element-1')).toEqual({ width: 150, height: 75 }); + }); + + it('removes a node that is not the latest without affecting the latest', () => { + const { add, observe, has, writes } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + const cleanupA = observe('element-1', nodeA); + observe('element-1', nodeB); + + cleanupA(); + + expect(lastObserver().observed.has(nodeB)).toBe(true); + expect(has('element-1')).toBe(true); + + lastObserver().report(nodeB, 300, 150); + expect(writes).toHaveBeenCalledTimes(1); + }); + + it('handles removing the latest of three nodes', () => { + const { add, observe, has } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + const nodeC = newNode(); + observe('element-1', nodeA); + observe('element-1', nodeB); + const cleanupC = observe('element-1', nodeC); + + cleanupC(); + + expect(lastObserver().observed.has(nodeC)).toBe(false); + expect(lastObserver().observed.has(nodeB)).toBe(true); + expect(lastObserver().observed.has(nodeA)).toBe(false); + expect(has('element-1')).toBe(true); + }); + + it('fully unregisters the element after all its nodes are cleaned up', () => { + const { add, observe, has } = setup(); + add('element-1'); + const cleanupA = observe('element-1'); + const cleanupB = observe('element-1'); + + cleanupB(); + expect(has('element-1')).toBe(true); + + cleanupA(); + expect(has('element-1')).toBe(false); + expect(lastObserver().observed.size).toBe(0); + }); + + it('uses the transform of the latest node, not a previous one', () => { + const { add, observe, size } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + const transformA = jest.fn(({ width, height }) => ({ width: width + 10, height: height + 10 })); + const transformB = jest.fn(({ width, height }) => ({ width: width + 50, height: height + 50 })); + observe('element-1', nodeA, transformA); + observe('element-1', nodeB, transformB); + + lastObserver().report(nodeB, 100, 50); + + expect(transformA).not.toHaveBeenCalled(); + expect(transformB).toHaveBeenCalled(); + expect(size('element-1')).toEqual({ width: 150, height: 100 }); + }); + + it('uses the previous transform after the latest node is removed', () => { + const { add, observe, size } = setup(); + add('element-1'); + const nodeA = newNode(); + const nodeB = newNode(); + const transformA = jest.fn(({ width, height }) => ({ width: width + 10, height: height + 10 })); + const transformB = jest.fn(({ width, height }) => ({ width: width + 50, height: height + 50 })); + observe('element-1', nodeA, transformA); + const cleanupB = observe('element-1', nodeB, transformB); + + cleanupB(); + lastObserver().report(nodeA, 100, 50); + + expect(transformA).toHaveBeenCalled(); + expect(transformB).not.toHaveBeenCalled(); + expect(size('element-1')).toEqual({ width: 110, height: 60 }); + }); + + it('has an idempotent cleanup', () => { + const { add, observe, has } = setup(); + add('element-1'); + const cleanup = observe('element-1'); + + cleanup(); + expect(has('element-1')).toBe(false); + + expect(() => cleanup()).not.toThrow(); + expect(has('element-1')).toBe(false); + }); + + it('handles interleaved registrations across different elements', () => { + const { add, observe, has } = setup(); + add('element-1'); + add('element-2'); + const node1A = newNode(); + const node1B = newNode(); + const node2A = newNode(); + const cleanup1A = observe('element-1', node1A); + observe('element-2', node2A); + observe('element-1', node1B); + + expect(lastObserver().observed.has(node1B)).toBe(true); + expect(lastObserver().observed.has(node2A)).toBe(true); + expect(lastObserver().observed.has(node1A)).toBe(false); + + cleanup1A(); + expect(lastObserver().observed.has(node1B)).toBe(true); + expect(has('element-1')).toBe(true); + expect(has('element-2')).toBe(true); + }); +}); + +describe('measurement — destroy', () => { + it('stops observing every element', () => { + const { measurement, add, observe, has } = setup(); + add('element-1'); + add('element-2'); + observe('element-1'); + observe('element-2'); + expect(has('element-1')).toBe(true); + expect(has('element-2')).toBe(true); + + measurement.destroy(); + + expect(has('element-1')).toBe(false); + expect(has('element-2')).toBe(false); + expect(lastObserver().isDisconnected).toBe(true); + }); + + it('stops observing elements with several nodes, and a late cleanup is a no-op', () => { + const { measurement, add, observe, has } = setup(); + add('element-1'); + add('element-2'); + observe('element-1'); + const cleanupLatest = observe('element-1'); + observe('element-2'); + + measurement.destroy(); + + expect(has('element-1')).toBe(false); + expect(has('element-2')).toBe(false); + expect(() => cleanupLatest()).not.toThrow(); + expect(lastObserver().observed.size).toBe(0); + }); + + it('stops following the graph', async () => { + const { measurement, add, version } = setup(); + measurement.destroy(); + + add('element-1'); + await flush(); + + expect(version()).toBe(0); + }); +}); + +describe('measurement — dev warning', () => { + it('warns once when an observed element is resized by the application, never for a measurement', () => { + const { add, observe } = setup(); + const observed = add('warned'); + const plain = add('plain'); + const node = newNode(); + const cleanup = observe('warned', node); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + plain.resize(9, 9); + lastObserver().report(node, 100, 50); + expect(warn).not.toHaveBeenCalled(); + + observed.resize(200, 80); + observed.resize(220, 90); + expect(warn).toHaveBeenCalledTimes(1); + + // No longer observed: its size is the application's to set. + cleanup(); + warn.mockClear(); + observed.resize(10, 10); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); + +const sized = (id: string) => ({ id, type: 'element', size: { width: 100, height: 50 } }); +const unsized = (id: string) => ({ id, type: 'element', size: { width: 0, height: 0 } }); +const link = (id: string, source: string, target: string) => ({ + id, + type: 'standard.Link', + source: { id: source }, + target: { id: target }, +}); + +// What `graph-changes.ts` used to forward as callbacks, now heard by the module itself. +describe('measurement — graph events', () => { + it('tracks each element a reset seeds, and changes once for them', async () => { + const { graph, version } = setup(); + + graph.resetCells([sized('a'), sized('b'), link('l1', 'a', 'b')]); + await flush(); + expect(version()).toBe(1); + + // Both were tracked: with one of them gone the diagram is still measured. + graph.getCell('a').remove(); + await flush(); + expect(version()).toBe(2); + }); + + it('clears what it tracked before tracking the seed of a reset', async () => { + const { graph, version, add } = setup(); + add('a', 100, 50); + await flush(); + expect(version()).toBe(1); + + // Nothing is measured from the reset until its seed settles. + graph.resetCells([sized('x')]); + expect(version()).toBe(0); + await flush(); + // A value is never reused, so the pass after a reset is seen as a change. + expect(version()).toBe(2); + + // A reset to no elements stays unmeasured. + graph.resetCells([link('l', 'x', 'x')]); + await flush(); + expect(version()).toBe(0); + }); + + it('changes when an element is removed, and not when a link is', async () => { + const { graph, version } = setup(); + graph.addCells([sized('a'), sized('b'), link('l1', 'a', 'b')]); + await flush(); + expect(version()).toBe(1); + + graph.getCell('l1').remove(); + await flush(); + expect(version()).toBe(1); + + graph.getCell('a').remove(); + await flush(); + expect(version()).toBe(2); + }); + + it('is not measured any more once the last element is removed', async () => { + const { graph, version } = setup(); + graph.addCells([sized('a'), unsized('z')]); + await flush(); + expect(version()).toBe(1); + + graph.getCell('a').remove(); + await flush(); + // A zero-sized element is still an element with a known size. + expect(version()).toBe(2); + + graph.getCell('z').remove(); + await flush(); + expect(version()).toBe(0); + }); + + it('changes for a measurement write, told apart by the change:size options', async () => { + const { graph, version } = setup(); + graph.resetCells([unsized('a')]); + await flush(); + expect(version()).toBe(1); + + (graph.getCell('a') as dia.Element).set( + 'size', + { width: 120, height: 60 }, + { [AUTO_SIZE_OPTION]: true } + ); + await flush(); + + expect(version()).toBe(2); + }); + + it('never changes for a size the application writes', async () => { + const { version, add } = setup(); + const element = add('a'); + await flush(); + + element.resize(70, 70); + element.resize(0, 0); + await flush(); + + expect(version()).toBe(1); + }); + + // Regression: elements already in the graph when the measurement is created + // (an external graph) were never counted, so the state was wrong from then on. + it('counts the elements the graph already holds', async () => { + const graph = new dia.Graph({}, { cellNamespace: DEFAULT_CELL_NAMESPACE }); + graph.addCells([sized('a'), sized('b'), link('l1', 'a', 'b')]); + const measurement = createMeasurement({ + graph, + autoSizeOrigin: 'top-left', + paperStores: NO_PAPERS, + }); + await flush(); + expect(measurement.getSizingVersion()).toBe(1); + + graph.getCell('a').remove(); + await flush(); + // One element is left, so the diagram is still measured. + expect(measurement.getSizingVersion()).toBe(2); + + graph.getCell('b').remove(); + await flush(); + expect(measurement.getSizingVersion()).toBe(0); + }); + + it('does not track links', async () => { + const { graph, version } = setup(); + + graph.resetCells([link('l1', 'a', 'b')]); + graph.addCell(link('l2', 'a', 'b')); + await flush(); + + expect(version()).toBe(0); + }); + + it('changes once for a batch of elements, whatever their sizes', async () => { + const { graph, version, add } = setup(); + graph.addCell(unsized('zero')); + add('a'); + add('b'); + await flush(); + + expect(version()).toBe(1); + }); +}); diff --git a/packages/joint-react/src/store/clear-view.ts b/packages/joint-react/src/store/clear-view.ts index 2fc483eebe..28ff1942d3 100644 --- a/packages/joint-react/src/store/clear-view.ts +++ b/packages/joint-react/src/store/clear-view.ts @@ -2,51 +2,6 @@ import type { dia } from '@joint/core'; import type { CellId } from '../types/cell.types'; import type { IncrementalChange } from '../state/incremental.types'; -/** - * Cache entry for batched clearView updates. - */ -export interface ClearViewCacheEntry { - /** Callback to validate which links should be cleared */ - onValidateLink?: (link: dia.Link) => boolean; -} - -/** - * Merges two clearView validators. - * - If no validator (clear all links), that takes precedence - * - If both have validators, creates union validator - * @param existing - Existing validator - * @param incoming - New validator - * @returns Merged validator or undefined - */ -export function mergeClearViewValidators( - existing: ClearViewCacheEntry | undefined, - incoming: ClearViewCacheEntry -): ClearViewCacheEntry { - // No existing entry - use new - if (!existing) { - return incoming; - } - - const { onValidateLink } = incoming; - - // No validator means clear all links - this takes precedence - if (!onValidateLink) { - return { onValidateLink: undefined }; - } - - // If existing has validator, create union - if (existing.onValidateLink) { - const existingValidator = existing.onValidateLink; - const newValidator = onValidateLink; - return { - onValidateLink: (link: dia.Link) => existingValidator(link) || newValidator(link), - }; - } - - // Existing has no validator (clear all) - keep that behavior - return existing; -} - /** * Determines if a link should be cleared based on cell connection. * @param link - The link to check @@ -105,32 +60,3 @@ export function clearConnectedLinkViews( } return changes; } - -/** - * Executes clearView for a single cell across all papers. - * @param papers - Iterable of paper stores with paper property - * @param graph - The JointJS Graph instance - * @param cellId - The cell ID to clear - * @param onValidateLink - Optional callback to determine which links to keep - */ -export function executeClearViewForCell( - papers: Iterable<{ readonly paper?: dia.Paper }>, - graph: dia.Graph, - cellId: CellId, - onValidateLink?: (link: dia.Link) => boolean -): void { - for (const paperStore of papers) { - const { paper } = paperStore; - if (!paper) { - continue; - } - - const elementView = paper.getCellView(cellId); - if (!elementView) { - continue; - } - - elementView.cleanNodesCache(); - clearConnectedLinkViews(paper, graph, cellId, onValidateLink); - } -} diff --git a/packages/joint-react/src/store/create-elements-size-observer.ts b/packages/joint-react/src/store/create-elements-size-observer.ts deleted file mode 100644 index 870c180aca..0000000000 --- a/packages/joint-react/src/store/create-elements-size-observer.ts +++ /dev/null @@ -1,383 +0,0 @@ -/** - * Element size observer with stack-based multi-hook support. - * - * Tracks DOM element sizes via ResizeObserver and syncs them to the graph. - * Multiple {@link useMeasureElement} hooks can target the same cell ID, only the - * most recently added (active) node is observed. When it unmounts, the - * previous node in the stack becomes active again. - * - * Internal data structures: - * - `observedStacksByCellId`, `Map` (last = active) - * - `activeObservedElementByDomNode`, `WeakMap` for O(1) lookup in the ResizeObserver callback - */ -import type { dia } from '@joint/core'; -import type { CellId } from '../types/cell.types'; -import type { ElementLayout } from '../types/cell.types'; -import type { ElementJSONInit } from '../types/cell.types'; - -const DEFAULT_OBSERVER_OPTIONS: ResizeObserverOptions = { box: 'border-box' }; -// Epsilon value to avoid jitter due to sub-pixel rendering -// especially on Safari -const EPSILON = 0.5; - -/** Element layout where width/height are required but x/y may be omitted. */ -type ElementLayoutOptionalXY = Pick & - Partial>; - -/** - * The element's measurement, passed to a {@link TransformElementLayout} callback. - * Carries the element's current `x`, `y`, and `angle` together with the freshly - * measured `width` and `height`, plus the underlying model and cell id. - * @expand - * @group Types - */ -export interface TransformElementLayoutParams extends Required { - /** The JointJS `dia.Element` instance being measured. */ - readonly model: dia.Element; - /** Id of the cell being measured. */ - readonly id: CellId; -} - -/** - * Adjusts a measured element layout before it is written to the graph. Receives - * the element's current geometry plus its newly measured size, and returns the - * `width`/`height` (and optionally `x`/`y`) to apply — use it to clamp sizes, - * snap to a grid, or reposition while auto-sizing. Pass it via the `transform` - * option of {@link useMeasureElement}. - * @example - * ```tsx - * import type { TransformElementLayout } from '@joint/react'; - * - * // Never let a measured element shrink below 80px wide. - * const transform: TransformElementLayout = ({ width, height }) => ({ - * width: Math.max(width, 80), - * height, - * }); - * ``` - * @see {@link TransformElementLayoutParams} - * @group Types - */ -export type TransformElementLayout = ( - params: TransformElementLayoutParams -) => ElementLayoutOptionalXY; - -/** - * Options for registering an element to be measured for size changes. - * @group Types - */ -export interface SetMeasuredNodeOptions { - /** The DOM node (HTML or SVG) to observe for size changes */ - readonly node: HTMLElement | SVGElement; - /** Optional callback to handle size updates before they're applied */ - readonly transform?: TransformElementLayout; - /** The ID of the cell in the graph that corresponds to this DOM node */ - readonly id: CellId; -} - -interface ObservedElement { - readonly id: CellId; - readonly node: HTMLElement | SVGElement; - readonly transform?: TransformElementLayout; - lastWidth?: number; - lastHeight?: number; - isMeasured: boolean; -} - -/** - * Options for creating an elements size observer. - */ -interface Options { - /** Options to pass to the ResizeObserver constructor */ - readonly resizeObserverOptions?: ResizeObserverOptions; - /** Function to get the current size of a cell from the graph */ - readonly getCellTransform: ( - id: CellId - ) => ElementLayoutOptionalXY & { model: dia.Element; angle: number }; - /** Function to get the elements from the container */ - readonly getElements: () => Map; - /** Callback function called when a batch of elements needs to be updated */ - readonly onBatchUpdate: (data: Record) => void; - /** - * An observed node was measured, whether or not the size differs from the - * model's (a measurement equal to the model size, or a zero size, writes - * nothing). Fires before `onBatchUpdate` for the same entries. - */ - readonly onElementMeasured?: (id: CellId) => void; -} - -/** - * Observer interface for tracking element size changes. - * Uses ResizeObserver to automatically detect when DOM elements change size - * and updates the corresponding graph elements. - * @group Types - */ -export interface GraphStoreObserver { - /** - * Adds an element to be observed for size changes. - * @param options - Options for measuring the element - * @returns Cleanup function to stop observing - */ - readonly add: (options: SetMeasuredNodeOptions) => () => void; - /** - * Cleans up all observers and resources. - */ - readonly clean: () => void; - /** - * Checks if a node is currently being observed. - * @param id - The ID of the cell to check - * @returns True if the node is being observed - */ - readonly has: (id: CellId) => boolean; -} - -/** - * Rounds a number to two decimals. - * @param value - The value to round to two decimals - * @returns The rounded value - */ -function roundToTwoDecimals(value: number) { - return Math.round(value * 100) / 100; -} - -/** - * Options for processing a single element's size change. - */ -interface ProcessSizeChangeOptions { - readonly measuredWidth: number; - readonly measuredHeight: number; - readonly observedElement: ObservedElement; - readonly getCellTransform: Options['getCellTransform']; - readonly elements: Map; - readonly mutableLayouts: Record; -} - -/** - * Processes a size change for a single element. - * Returns true if the element was updated, false otherwise. - * @param options - The options containing size data and element references - * @returns True if the element was updated, false otherwise - */ -function processSizeChange(options: ProcessSizeChangeOptions): boolean { - const { - measuredWidth, - measuredHeight, - observedElement, - getCellTransform, - elements, - mutableLayouts, - } = options; - - if (!elements.has(observedElement.id)) { - return false; - } - - const currentCellTransform = getCellTransform(observedElement.id); - if ( - Math.abs(currentCellTransform.width - measuredWidth) <= EPSILON && - Math.abs(currentCellTransform.height - measuredHeight) <= EPSILON - ) { - return false; - } - - if ( - Math.abs((observedElement.lastWidth ?? 0) - measuredWidth) <= EPSILON && - Math.abs((observedElement.lastHeight ?? 0) - measuredHeight) <= EPSILON - ) { - return false; - } - - observedElement.lastWidth = measuredWidth; - observedElement.lastHeight = measuredHeight; - - const { transform } = observedElement; - if (transform) { - // Provide the full current transform to the callback - const { x = 0, y = 0, angle = 0, model } = currentCellTransform; - mutableLayouts[observedElement.id] = transform({ - x, - y, - angle, - model, - width: measuredWidth, - height: measuredHeight, - id: observedElement.id, - }); - } else { - // No transform provided, use measured size directly. Position is left unchanged. - mutableLayouts[observedElement.id] = { - width: measuredWidth, - height: measuredHeight, - }; - } - - return true; -} - -/** - * Creates an observer for element size changes using the ResizeObserver API. - * - * This function sets up automatic size tracking for DOM elements that correspond to graph elements. - * When a DOM element's size changes (e.g., due to content changes or CSS updates), the observer - * automatically updates the corresponding graph element's size. - * - * **Features:** - * - Uses ResizeObserver for efficient size tracking - * - Compares sizes with epsilon to avoid jitter from sub-pixel rendering - * - Supports custom size update handlers - * @param options - The options for creating the size observer - * @returns A GraphStoreObserver instance with methods to add/remove observers - */ -export function createElementsSizeObserver(options: Options): GraphStoreObserver { - const { - resizeObserverOptions = DEFAULT_OBSERVER_OPTIONS, - getCellTransform, - onBatchUpdate, - onElementMeasured, - getElements, - } = options; - - // Stack per cell ID: last entry is the active (observed) node. - // When the active node unmounts, the previous one becomes active again. - const observedStacksByCellId = new Map(); - - // Maps only the active DOM node to its ObservedElement for O(1) lookup in the ResizeObserver callback. - const activeObservedElementByDomNode = new WeakMap(); - - /** Returns the active (last) element from the stack, or `undefined` if empty. */ - function getActiveElement(stack: readonly ObservedElement[]): ObservedElement | undefined { - return stack.at(-1); - } - - /** Starts observing the given element and registers it in the active DOM node lookup. */ - function activateElement(observedElement: ObservedElement) { - observer.observe(observedElement.node, resizeObserverOptions); - activeObservedElementByDomNode.set(observedElement.node, observedElement); - } - - /** Stops observing the given element and removes it from the active DOM node lookup. */ - function deactivateElement(observedElement: ObservedElement) { - observer.unobserve(observedElement.node); - activeObservedElementByDomNode.delete(observedElement.node); - } - - const observer = new ResizeObserver((entries) => { - let hasAnySizeChange = false; - const elements = getElements(); - const mutableLayouts: Record = {}; - - for (const entry of entries) { - const { target, borderBoxSize } = entry; - - const observedElement = activeObservedElementByDomNode.get( - target as HTMLElement | SVGElement - ); - if (!observedElement) continue; - - onElementMeasured?.(observedElement.id); - - if (!borderBoxSize || borderBoxSize.length === 0) { - continue; - } - - const [size] = borderBoxSize; - const { inlineSize, blockSize } = size; - - const measuredWidth = roundToTwoDecimals(inlineSize); - const measuredHeight = roundToTwoDecimals(blockSize); - - // Skip zero-size entries. This happens when an element is hidden (display:none) - // or removed from the DOM. We must never propagate 0-size to the model. - if (measuredWidth <= 0 || measuredHeight <= 0) { - continue; - } - - const wasUpdated = processSizeChange({ - measuredWidth, - measuredHeight, - observedElement, - getCellTransform, - mutableLayouts, - elements, - }); - - if (wasUpdated) { - hasAnySizeChange = true; - } - } - - if (!hasAnySizeChange) { - return; - } - - onBatchUpdate(mutableLayouts); - }); - - return { - add({ id, node, transform }: SetMeasuredNodeOptions) { - const observedElement: ObservedElement = { - id, - node, - transform, - isMeasured: false, - }; - - let stack = observedStacksByCellId.get(id); - - if (!stack) { - stack = []; - observedStacksByCellId.set(id, stack); - } - - // Deactivate the current active node before pushing the new one - const previousActive = getActiveElement(stack); - if (previousActive) { - deactivateElement(previousActive); - } - - stack.push(observedElement); - activateElement(observedElement); - - return () => { - const currentStack = observedStacksByCellId.get(id); - if (!currentStack) return; - - const isActive = getActiveElement(currentStack) === observedElement; - - if (isActive) { - deactivateElement(observedElement); - currentStack.pop(); - - // Fall back to the previous node in the stack - const newActive = getActiveElement(currentStack); - if (newActive) { - activateElement(newActive); - } - } else { - // Remove non-active entry from the middle of the stack - const index = currentStack.indexOf(observedElement); - if (index !== -1) { - currentStack.splice(index, 1); - } - } - - if (currentStack.length === 0) { - observedStacksByCellId.delete(id); - } - }; - }, - clean() { - // cleanup all observed nodes and clear all stacks - for (const [, stack] of observedStacksByCellId.entries()) { - for (const { node } of stack) { - observer.unobserve(node); - } - } - observedStacksByCellId.clear(); - observer.disconnect(); - }, - has(id: CellId) { - const stack = observedStacksByCellId.get(id); - return !!stack && stack.length > 0; - }, - }; -} diff --git a/packages/joint-react/src/store/graph-changes.ts b/packages/joint-react/src/store/graph-changes.ts index e25fe70cf8..3d87a4d6e1 100644 --- a/packages/joint-react/src/store/graph-changes.ts +++ b/packages/joint-react/src/store/graph-changes.ts @@ -60,27 +60,7 @@ interface OnChangeOptions { readonly isReset?: boolean; } -/** - * Graph events the measurement bookkeeping in `GraphStore` listens to. Forwarded - * unchanged by `graphProjection`. - */ -export interface MeasurementListeners { - /** - * An element got a size. `changeOptions` are the options of the `change:size` - * event; `undefined` when the size arrives with the cell (`add` / `reset`). - */ - readonly onElementsSizeChange?: ( - id: CellId, - size: dia.Size, - changeOptions?: dia.Cell.Options - ) => void; - /** An element left the graph (not fired by a `reset`; see {@link onReset}). */ - readonly onElementRemove?: (id: CellId) => void; - /** The graph was reset; fires before the seed cells' `onElementsSizeChange` calls. */ - readonly onReset?: () => void; -} - -interface Options extends MeasurementListeners { +interface Options { readonly graph: dia.Graph; readonly onChanges: (options: OnChangeOptions) => void; } @@ -97,7 +77,7 @@ interface JointJSEventOptions { * @returns Controller exposing updateGraph and destroy. */ export function graphChanges(options: Options) { - const { graph, onElementsSizeChange, onElementRemove, onReset } = options; + const { graph } = options; const changes = new Map>(); let batchDepth = 0; @@ -168,10 +148,6 @@ export function graphChanges(options: Options) { _collection: mvc.Collection, { isUpdateFromReact }: JointJSEventOptions ) => { - if (cell.isElement() && onElementsSizeChange) { - const size = cell.size(); - onElementsSizeChange(cell.id, size); - } if (isUpdateFromReact) return; onCellEvent(cell, 'add'); } @@ -184,7 +160,6 @@ export function graphChanges(options: Options) { _collection: mvc.Collection, { isUpdateFromReact }: JointJSEventOptions ) => { - if (cell.isElement()) onElementRemove?.(cell.id); if (isUpdateFromReact) return; onCellEvent(cell, 'remove'); } @@ -196,16 +171,8 @@ export function graphChanges(options: Options) { if (eventOptions.isUpdateFromReact) return; isSyncedWithReact = true; changes.clear(); - onReset?.(); for (const cell of collection.models) { changes.set(cell.id, { type: 'add', data: cell }); - // `reset` suppresses per-cell `add` events, so size notifications - // for seed elements never reach `onElementsSizeChange` via the - // `add` listener. Mirror that path here so initial-measurement - // gating (e.g. `useOnElementsMeasured`) fires for seed cells. - if (onElementsSizeChange && cell.isElement()) { - onElementsSizeChange(cell.id, (cell as dia.Element).size()); - } } // Bypass the simpleScheduler wrapper used for normal cell events. // `reset` is a one-shot bulk operation and callers (e.g. GraphStore @@ -224,15 +191,6 @@ export function graphChanges(options: Options) { onChanges({ changes: layoutChanges, isInsideBatch: true, deferCommit: isDeferring() }); }); - controller.listenTo( - graph, - 'change:size', - (cell: dia.Cell, newSize: dia.Size, changeOptions: dia.Cell.Options = {}) => { - if (!onElementsSizeChange) return; - onElementsSizeChange(cell.id, newSize, changeOptions); - } - ); - // Always-on batch tracking. A batch flagged with DEFER_COMMIT_BATCH_OPTION // defers its container commits until it closes, so a burst of edits (sync or // async) flushes as a single React update. Plain batches commit live. diff --git a/packages/joint-react/src/store/graph-projection.ts b/packages/joint-react/src/store/graph-projection.ts index 02578ac446..01996c7cc1 100644 --- a/packages/joint-react/src/store/graph-projection.ts +++ b/packages/joint-react/src/store/graph-projection.ts @@ -1,6 +1,6 @@ import { type dia } from '@joint/core'; import type { ElementJSONInit, LinkJSONInit, CellId } from '../types/cell.types'; -import { graphChanges, type MeasurementListeners, type UpdateGraphOptions } from './graph-changes'; +import { graphChanges, type UpdateGraphOptions } from './graph-changes'; import { asReadonlyContainer, createContainer, type ContainerChangeSet } from './state-container'; import { mergeCellRecord, toCellRecord } from '../state/data-mapping/cell-record-merge'; @@ -36,7 +36,7 @@ export type OnIncrementalCellsChange extends MeasurementListeners { +> { readonly graph: dia.Graph; readonly onIncrementalCellsChange?: OnIncrementalCellsChange; } @@ -55,8 +55,7 @@ export function graphProjection< Element extends ElementJSONInit = ElementJSONInit, Link extends LinkJSONInit = LinkJSONInit, >(options: GraphProjectionState) { - const { graph, onIncrementalCellsChange, onElementsSizeChange, onElementRemove, onReset } = - options; + const { graph, onIncrementalCellsChange } = options; const cells = createContainer(); @@ -123,9 +122,6 @@ export function graphProjection< const graphChangesController = graphChanges({ graph, - onElementsSizeChange, - onElementRemove, - onReset, onChanges: ({ changes, isInsideBatch, deferCommit, isReset }) => { // Elements removed in this batch — swept once after the loop for link // records they may have stranded. @@ -236,6 +232,12 @@ export function graphProjection< } } + // Cleared before anything is notified: a subscriber may change the graph + // from its callback, and those changes are recorded here for the next + // pass. Clearing afterwards would drop them and leave the store out of + // sync with the graph. + changes.clear(); + // Two commit modes, decided by `deferCommit` (see graph-changes): // // - Plain batches (interactive drags, `auto-size`, layout) and lone edits @@ -261,8 +263,6 @@ export function graphProjection< changed!.clear(); removed!.clear(); } - - changes.clear(); }, }); diff --git a/packages/joint-react/src/store/graph-state.ts b/packages/joint-react/src/store/graph-state.ts new file mode 100644 index 0000000000..4d20a8e8b9 --- /dev/null +++ b/packages/joint-react/src/store/graph-state.ts @@ -0,0 +1,339 @@ +import { mvc, type dia } from '@joint/core'; +import type { AnyCellRecord, CellId, CellRecord, Computed } from '../types/cell.types'; +import type { GraphStore } from './graph-store'; +import type { ReadonlyContainer } from './state-container'; +import { toCellRecord } from '../state/data-mapping/cell-record-merge'; +import { isCollection } from '../utils/is'; +import { warnGraphStateReadOutsideSelector, warnUnstableSelector } from '../utils/dev-warnings'; +import { arrayAwareEqual } from '../selectors/selector-utils'; + +/** + * The state of a graph, as a selector of {@link useGraphState} sees it. Read + * it inside the selector only: the hook subscribes to exactly what the + * selector read, so a selector that reads one cell is not run when another + * cell changes, and one that reads `sizingVersion` is not run on a drag. + * @template Cell - Input cell record shape (defaults to CellRecord); reads resolve to its Computed form. + * @internal + */ +export interface GraphState { + /** All cells. A new array on every graph change, so a selector reading it runs on each of them. */ + readonly cells: ReadonlyArray>; + /** + * One cell by id, or `undefined` when the graph does not hold it. + * @param id - The id of the cell to read. + */ + readonly getCell: (id: CellId) => Computed | undefined; + /** + * The cells with the given ids, or the cells of a JointJS collection, in + * that order. Cells the graph does not hold are skipped, except the members + * of a collection, which are read from the collection itself. + * @param target - Cell ids, or a `mvc.Collection` of cells. + */ + readonly getCells: ( + target: readonly CellId[] | mvc.Collection + ) => ReadonlyArray>; + /** + * `0` while the element sizes are not known, otherwise a number that changes + * each time they settle. See {@link createMeasurement}. + */ + readonly sizingVersion: number; +} + +/** + * Derives a value from the {@link GraphState}. + * @internal + */ +export type GraphStateSelector = ( + state: GraphState +) => Selected; + +/** + * Equality test that tells whether a newly selected value is a change. + * @group Types + */ +export type SelectedEqual = (a: Selected, b: Selected) => boolean; + +const ALL_CELLS = Symbol('cells'); +const SIZING_VERSION = Symbol('sizingVersion'); +/** Membership and order of a collection; `sort` reorders without adding or removing. */ +const COLLECTION_EVENTS = 'update reset sort'; + +/** A part of the graph state a selector can read: all cells, the sizing version, a collection or one cell. */ +type Source = typeof ALL_CELLS | typeof SIZING_VERSION | mvc.Collection | CellId; + +/** The parts of a {@link GraphStore} the reader reads, whatever its cell shapes are. */ +interface GraphStateStore { + readonly graphProjection: { readonly cells: ReadonlyContainer }; + readonly measurement: Pick; +} + +interface Subscription { + readonly listener: () => void; + readonly unsubscribers: Map void>; +} + +const collectionIds = new WeakMap, readonly CellId[]>(); +const detachedRecords = new WeakMap(); + +/** + * The ids of a collection's cells, the same array for as long as the + * membership and order are. O(members) per call, nothing allocated while unchanged. + * @param collection - The collection to read. + */ +function getCollectionIds(collection: mvc.Collection): readonly CellId[] { + const { models } = collection; + const known = collectionIds.get(collection); + if (known?.length === models.length) { + let isSame = true; + for (const [index, model] of models.entries()) { + if (model.id !== known[index]) { + isSame = false; + break; + } + } + if (isSame) return known; + } + const ids = models.map((model) => model.id); + collectionIds.set(collection, ids); + return ids; +} + +/** + * The record of a collection member the graph does not hold (a `ui.Clipboard` + * clone, for example), one per cell so repeated reads share a reference. + * @param collection - The collection the cell is a member of. + * @param id - The id of the cell. + */ +function getDetachedRecord( + collection: mvc.Collection, + id: CellId +): AnyCellRecord | undefined { + const cell = collection.get(id); + if (!cell) return undefined; + let record = detachedRecords.get(cell); + if (!record) { + record = toCellRecord(cell); + detachedRecords.set(cell, record); + } + return record; +} + +/** + * Reads a selected value from a graph store and keeps track of what the + * selector read, so that it is run again, and its subscriber notified, only + * when one of those parts changes. One reader serves one hook instance. + * + * The staleness test compares what the selector saw with the store itself, not + * with what the subscription delivered, so a change that lands between a + * render and its subscription is not missed. + * @param store - The graph store to read. + * @param hookName - The hook the application called, named in dev warnings. + * @returns `read`, `sync` and `subscribe`, all identity-stable. + * @internal + */ +export function createGraphStateReader( + store: GraphStateStore, + hookName = 'useGraphState' +) { + // The store holds resolved (Computed) records of the application's own cell + // shape; the `Cell` generic is how the caller declares that shape. + const cells = store.graphProjection.cells as ReadonlyContainer>; + const { measurement } = store; + // What the last selector run read, with the value it saw. + const seen = new Map(); + let subscription: Subscription | undefined; + // The selector `seen` belongs to. Unset from the start of a run until it + // returns, so a selector that throws leaves nothing to trust. + let lastSelector: GraphStateSelector | undefined; + let selected: { readonly value: Selected } | undefined; + // True only while a selector runs: the state is not to be read at any other time. + let isSelecting = false; + // True while the subscription listens to exactly what `seen` holds. Then a + // change cannot go unreported, and `isDirty` says whether one was: a read + // needs no scan of `seen`. Without it, k changed cells out of N watched ones + // cost k scans of N per commit. + let isListeningToSeen = false; + let isDirty = false; + + const current = (source: Source): unknown => { + if (source === ALL_CELLS) return cells.getSnapshot(); + if (source === SIZING_VERSION) return measurement.getSizingVersion(); + if (typeof source === 'object') return getCollectionIds(source); + return cells.get(source); + }; + + const subscribeTo = (source: Source, notify: () => void): (() => void) => { + // Compares its own source only: once a selector run has seen the new + // values, the rest of the notifications of the same commit find no change. + const listener = () => { + if (!Object.is(current(source), seen.get(source))) isDirty = true; + notify(); + }; + if (source === ALL_CELLS) return cells.subscribe(listener); + if (source === SIZING_VERSION) return measurement.subscribe(listener); + if (typeof source === 'object') { + const controller = new mvc.Listener(); + controller.listenTo(source, COLLECTION_EVENTS, listener); + return () => controller.stopListening(); + } + return cells.subscribeById(source, listener); + }; + + const hasChanged = (): boolean => { + if (isListeningToSeen && !isDirty) return false; + for (const [source, value] of seen) { + if (!Object.is(current(source), value)) return true; + } + isDirty = false; + return false; + }; + + /** Makes the subscription listen to what the last run read, and to nothing else. */ + const syncSubscription = ({ listener, unsubscribers }: Subscription): void => { + for (const [source, unsubscribe] of unsubscribers) { + if (seen.has(source)) continue; + unsubscribe(); + unsubscribers.delete(source); + } + for (const source of seen.keys()) { + if (unsubscribers.has(source)) continue; + unsubscribers.set(source, subscribeTo(source, listener)); + // It may have changed between being read and being listened to. + isDirty = true; + } + isListeningToSeen = true; + }; + + /** Records what a selector read. Outside of a selector run nothing would follow it. */ + const see = (source: Source, value: unknown): void => { + if (isSelecting) seen.set(source, value); + else warnGraphStateReadOutsideSelector(); + }; + + const getCell = (id: CellId): Computed | undefined => { + const cell = cells.get(id); + see(id, cell); + return cell; + }; + + const state: GraphState = { + get cells() { + const snapshot = cells.getSnapshot(); + see(ALL_CELLS, snapshot); + return snapshot; + }, + get sizingVersion() { + const sizingVersion = measurement.getSizingVersion(); + see(SIZING_VERSION, sizingVersion); + return sizingVersion; + }, + getCell, + getCells(target) { + let collection: mvc.Collection | undefined; + let ids: readonly CellId[]; + if (isCollection(target)) { + collection = target; + ids = getCollectionIds(target); + see(target, ids); + } else { + ids = target; + } + const picked: Array> = []; + for (const id of ids) { + // A detached record is of the application's cell shape too, like the + // records of the container above. + const cell = + getCell(id) ?? + (collection && (getDetachedRecord(collection, id) as Computed | undefined)); + if (cell !== undefined) picked.push(cell); + } + return picked; + }, + }; + + const read = ( + selector: GraphStateSelector, + isEqual: SelectedEqual = arrayAwareEqual + ): Selected => { + if (selected && selector === lastSelector && !hasChanged()) return selected.value; + lastSelector = undefined; + isListeningToSeen = false; + seen.clear(); + // Stays set if the selector throws: a read outside of a selector then goes + // unreported until the next run, which is all that is lost. + isSelecting = true; + const next = selector(state); + isSelecting = false; + lastSelector = selector; + isDirty = false; + if (selected) { + if (isEqual(selected.value, next)) return selected.value; + warnUnstableSelector(hookName, selected.value, next, isEqual !== arrayAwareEqual); + } + selected = { value: next }; + return next; + }; + + return { + /** + * The value `selector` selects. The selector runs when it is a new function + * or when something its last run read has changed; otherwise, and whenever + * the new value is equal to the last one, the last value is returned as is. + * + * Safe to call while rendering: it leaves the subscription alone, so a + * render React throws away cannot move it. See {@link sync}. + * @param selector - derives the value from the graph state + * @param isEqual - tells whether a newly selected value is a change + * @returns the selected value, reference-stable while it is equal + */ + read, + /** + * Reads like {@link read}, then makes the subscription listen to what that + * run read. Call it with the selector that is in effect, outside of render: + * once it has committed, or when a notification arrives. + * @param selector - derives the value from the graph state + * @param isEqual - tells whether a newly selected value is a change + * @returns the selected value, reference-stable while it is equal + */ + sync( + selector: GraphStateSelector, + isEqual?: SelectedEqual + ): Selected { + const value = read(selector, isEqual); + if (subscription && lastSelector && !isListeningToSeen) syncSubscription(subscription); + return value; + }, + /** + * Calls `listener` when something the selector read may have changed. The + * listener is expected to read again with the selector in effect; the + * subscription then moves to whatever that run read. + * @param listener - called on a change, synchronously with the store + * @returns the unsubscribe function + */ + subscribe(listener: () => void): () => void { + const own: Subscription = { + listener: () => { + listener(); + // The listener has read with the selector in effect: follow it. Not + // after unsubscribing, which the listener itself may have done. + if (subscription === own && lastSelector && !isListeningToSeen) syncSubscription(own); + }, + unsubscribers: new Map(), + }; + subscription = own; + syncSubscription(own); + return () => { + for (const unsubscribe of own.unsubscribers.values()) unsubscribe(); + own.unsubscribers.clear(); + if (subscription !== own) return; + subscription = undefined; + isListeningToSeen = false; + }; + }, + }; +} + +/** Reader returned by {@link createGraphStateReader}. */ +export type GraphStateReader = ReturnType< + typeof createGraphStateReader +>; diff --git a/packages/joint-react/src/store/graph-store.ts b/packages/joint-react/src/store/graph-store.ts index 22d745f31f..e1837b4a76 100644 --- a/packages/joint-react/src/store/graph-store.ts +++ b/packages/joint-react/src/store/graph-store.ts @@ -3,17 +3,12 @@ import type { ElementJSONInit, LinkJSONInit, CellId } from '../types/cell.types' import type { AddPaperOptions } from './paper-store'; import { PaperStore, getDefaultPaperState } from './paper-store'; -import { - createElementsSizeObserver, - type GraphStoreObserver, - type SetMeasuredNodeOptions, -} from './create-elements-size-observer'; +import { createMeasurement, type AutoSizeOrigin, type Measurement } from './measurement'; import { ELEMENT_MODEL_TYPE, ElementModel } from '../mvc/element-model'; import { LINK_MODEL_TYPE, LinkModel } from '../mvc/link-model'; import { isElementType, isLinkType } from '../utils/cell-type'; import { clearConnectedLinkViews } from './clear-view'; import { LAYOUT_UPDATE_EVENT } from './graph-changes'; -import { isPaperView } from '../mvc/paper'; import { createAtom, type Atom } from './state-container'; import type { IncrementalChange } from '../state/incremental.types'; import type { Feature } from '../types/feature.types'; @@ -22,10 +17,9 @@ import { type GraphProjection, type OnIncrementalCellsChange, } from './graph-projection'; -import { simpleScheduler } from '../utils/scheduler'; import { cellInputToModel } from '../utils/normalize-cell-input'; import type { CellInput } from '../types/cell.types'; -import { warnDuplicatePapers, warnResizeOnAutoSizedElement } from '../utils/dev-warnings'; +import { warnDuplicatePapers } from '../utils/dev-warnings'; export const DEFAULT_CELL_NAMESPACE: Record = { ...shapes, @@ -33,18 +27,6 @@ export const DEFAULT_CELL_NAMESPACE: Record = { [LINK_MODEL_TYPE]: LinkModel, }; -/** - * `dia.Cell.set()` option key used to mark writes that originate from the - * auto-size / measurement pipeline. `change:size` listeners can read it to - * distinguish measurement writes from external ones (controlled-mode sync, - * `cell.resize`, interactive tools) and avoid feedback loops. Exported for - * plugin authors and `@joint/react-plus` via `@joint/react/internal`. - */ -export const AUTO_SIZE_OPTION = 'autoSize'; - -/** Option shape accepted by `dia.Cell.set()` writes from the auto-size pipeline. */ -type AutoSizeOptions = { [AUTO_SIZE_OPTION]?: boolean }; - /** * Paper snapshot is a simple version counter. * Incremented on every view mount/unmount change to trigger React re-renders. @@ -65,19 +47,6 @@ export interface GraphStoreInternalSnapshot { readonly graphFeaturesVersion: number; } -/** - * Reference point that stays fixed when an auto-sized element's measured size - * changes. Mirrors CSS `transform-origin` semantics. - * - * - `'top-left'` (default): element grows right/down, top-left stays put. - * - `'center'`: element grows symmetrically, geometric center stays put. - * - * Only affects writes from the {@link useMeasureElement} pipeline. Manual `cell.resize()`, - * interactive resize tools, and direct `cell.set('size', ...)` calls are unaffected. - * @group Types - */ -export type AutoSizeOrigin = 'top-left' | 'center'; - /** * Options for constructing a {@link GraphStore}: an optional existing `dia.Graph`, * cell namespace/model overrides, the auto-size origin, and `initialCells` used to @@ -109,24 +78,13 @@ export class GraphStore< > { public readonly graphProjection: GraphProjection; public readonly internalState: Atom; - public readonly measureState: Atom = createAtom(0); - /** - * Incremented by every graph `reset`. `useOnElementsMeasured` reports - * `isInitial` for the first `measureState` bump of each generation. - */ - public measureGeneration = 0; + /** Element measuring and its "sizes are settled" signal. See {@link createMeasurement}. */ + public readonly measurement: Measurement; public readonly graph: dia.Graph; - public readonly autoSizeOrigin: AutoSizeOrigin; public paperStores = new Map(); public features: Record = {}; - private observer: GraphStoreObserver; - /** Elements added to the graph that no paper has accounted for yet (see the constructor). */ - private readonly outstandingElements = new Set(); - private readonly scheduleMeasurementDelivery: () => void; private onIncrementalCellsChange?: OnIncrementalCellsChange; - // dev-only `change:size` listener that warns about resizing auto-sized elements. - private warnAutoSizeResize?: (cell: dia.Cell, size: dia.Size, opt?: AutoSizeOptions) => void; constructor(public readonly config: GraphStoreOptions) { const { @@ -136,7 +94,6 @@ export class GraphStore< autoSizeOrigin = 'top-left', initialCells, } = config; - this.autoSizeOrigin = autoSizeOrigin; this.graph = graph ?? @@ -157,126 +114,17 @@ export class GraphStore< graphFeaturesVersion: 1, }); - // Measurement bookkeeping behind `measureState` (see `useOnElementsMeasured`): - // one bump per settled change. An element is outstanding from arriving in - // the graph until it is accounted for: its React content committed without - // registering a measurer (`markElementRendered`), no paper renders it - // (`settleUnrenderedElements`, after a paper's render pass), or, when it did - // register one (`useMeasureElement`), the observer measured it. A zero size - // is never read as "waiting": it is a legal final size (a layout anchor). - // Sizes the application writes are not measurements and never bump (#3514). - const sizedElements = new Set(); - let hasUndeliveredChange = false; - const deliverMeasurement = () => { - if (!hasUndeliveredChange) return; - // Papers account for outstanding elements; without one nothing renders - // or measures, so there is nothing to wait for. - if (this.outstandingElements.size > 0 && this.paperStores.size > 0) return; - // "Measured" means at least one element has a size (`isInitial` contract). - if (sizedElements.size === 0) return; - hasUndeliveredChange = false; - this.measureState.set((previous) => previous + 1); - }; - this.scheduleMeasurementDelivery = () => simpleScheduler(deliverMeasurement); - this.graphProjection = graphProjection({ graph: this.graph, onIncrementalCellsChange: (changes) => { this.onIncrementalCellsChange?.(changes); }, - onElementsSizeChange: (id, size, changeOptions) => { - if (size.width > 0 && size.height > 0) { - sizedElements.add(id); - } else { - sizedElements.delete(id); - } - if (changeOptions === undefined) { - // The size arrived with the cell (`add` / `reset`): outstanding until - // a paper has rendered it, whatever the size says. - this.outstandingElements.add(id); - } else if (changeOptions[AUTO_SIZE_OPTION]) { - this.outstandingElements.delete(id); - } else { - return; - } - hasUndeliveredChange = true; - this.scheduleMeasurementDelivery(); - }, - onElementRemove: (id) => { - sizedElements.delete(id); - if (this.outstandingElements.delete(id)) this.scheduleMeasurementDelivery(); - }, - onReset: () => { - // The reset replaces the diagram: its first settled pass is a new - // `isInitial` for `useOnElementsMeasured`, and until then nothing is - // measured for `useAreElementsMeasured`. - sizedElements.clear(); - this.outstandingElements.clear(); - hasUndeliveredChange = false; - this.measureGeneration += 1; - this.measureState.set(0); - }, }); - - this.observer = createElementsSizeObserver({ - onElementMeasured: (id) => { - // Settles an element whose measurement equals the size it already has - // (the application pre-sized it): the observer then writes nothing. - if (this.outstandingElements.delete(id)) this.scheduleMeasurementDelivery(); - }, - getElements: () => { - // The observer only cares about element-typed cells. Build a Map on - // demand from the unified cells container — cold path, called only - // when the ResizeObserver fires. - const map = new Map(); - for (const cell of this.graphProjection.cells.getSnapshot()) { - if (cell.id === undefined) continue; - if (this.isElement(cell)) { - map.set(cell.id, cell); - } - } - return map; - }, - onBatchUpdate: (updatedElements) => { - this.graph.startBatch('auto-size'); - for (const [id, data] of Object.entries(updatedElements)) { - const model = this.graph.getCell(id); - if (!model?.isElement()) continue; - // `autoSize: true` marks writes that originate from the - // ResizeObserver pipeline so `change:size` listeners can tell our - // own writes apart from external ones (controlled-mode sync, - // direct `cell.resize`, etc.) and avoid feedback loops. - const attributes: dia.Cell.Attributes = { - size: { width: data.width, height: data.height }, - }; - - if (data.x !== undefined && data.y !== undefined) { - attributes.position = { x: data.x, y: data.y }; - } else if (this.autoSizeOrigin === 'center') { - // Center-anchored auto-size: keep the geometric center fixed. - const center = model.getCenter(); - attributes.position = { - x: center.x - data.width / 2, - y: center.y - data.height / 2, - }; - } - - model.set(attributes, { [AUTO_SIZE_OPTION]: true }); - // Top-left auto-size (default): don't write position — the cell's - // top-left stays put implicitly and it grows right/down. - } - this.graph.stopBatch('auto-size'); - }, - getCellTransform: (id) => { - const model = this.graph.getCell(id); - if (!model?.isElement()) throw new Error('Cell not found or not an element: ' + id); - return { - model, - ...model.size(), - ...model.position().toJSON(), - angle: model.angle(), - }; - }, + // Before the seed below, so the seed cells are tracked. + this.measurement = createMeasurement({ + graph: this.graph, + autoSizeOrigin, + paperStores: this.paperStores, }); if (initialCells && initialCells.length > 0) { @@ -292,19 +140,6 @@ export class GraphStore< // (e.g. stencil drag's cloneView). this.graphProjection.syncFromGraph(); } - - // dev only — warn when an auto-sized element (registered with the size - // observer because it renders without `useModelGeometry`) is resized by - // something other than the measurement pipeline. Such resizes are - // immediately overwritten by the measured content size. - if (process.env.NODE_ENV !== 'production') { - this.warnAutoSizeResize = (cell, _size, opt) => { - if (opt?.[AUTO_SIZE_OPTION]) return; // our own measurement write - if (!this.observer.has(cell.id)) return; // not auto-sized → resize is honored - warnResizeOnAutoSizedElement(cell.id); - }; - this.graph.on('change:size', this.warnAutoSizeResize); - } } public setOnIncrementalCellsChange = (callback: OnIncrementalCellsChange) => { @@ -370,10 +205,7 @@ export class GraphStore< this.paperStores.clear(); this.graphProjection.destroy(); this.internalState.clean(); - this.observer.clean(); - if (this.warnAutoSizeResize) { - this.graph.off('change:size', this.warnAutoSizeResize); - } + this.measurement.destroy(); if (!isGraphExternal) { this.graph.clear(); } @@ -465,6 +297,7 @@ export class GraphStore< const paperStore = this.paperStores.get(id); paperStore?.destroy(); this.paperStores.delete(id); + this.measurement.onPaperRemoved(); this.internalState.setState((previous) => { const newPapers: Record = {}; for (const [key, value] of Object.entries(previous.papers)) { @@ -484,53 +317,6 @@ export class GraphStore< return { paperStore, remove: () => this.removePaper(id) }; }; - public setMeasuredNode = (options: SetMeasuredNodeOptions) => this.observer.add(options); - - /** - * An element's React content committed. Called by the element portal item - * after its subtree's layout effects, so an element that registered for - * measurement (`useMeasureElement`) stays outstanding until measured, while - * one that nothing measures is settled now. - * @param id - the rendered element - */ - public markElementRendered = (id: CellId) => { - if (!this.outstandingElements.has(id) || this.observer.has(id)) return; - this.outstandingElements.delete(id); - this.scheduleMeasurementDelivery(); - }; - - /** - * A paper finished a render pass (`render:done`): every view it mounts for - * this pass is in the DOM now. An outstanding element that no paper renders - * (viewport culling, `cellVisibility`, a hidden group) is not going to be - * measured, so it is settled with the size it has; one with a mounted view - * waits for its content to commit (`markElementRendered`). O(outstanding), - * which is empty between changes. - */ - public settleUnrenderedElements = () => { - let didSettle = false; - for (const id of this.outstandingElements) { - if (this.observer.has(id) || this.isRenderedByAnyPaper(id)) continue; - this.outstandingElements.delete(id); - didSettle = true; - } - if (didSettle) this.scheduleMeasurementDelivery(); - }; - - /** - * Whether some paper mounted the element's view and will render React - * content into it, which is the only way a measurer gets registered: the - * paper has a `renderElement` and the view has a portal node. - */ - private isRenderedByAnyPaper(id: CellId): boolean { - for (const paperStore of this.paperStores.values()) { - if (!paperStore.renderElement) continue; - const view = paperStore.getElementView(id); - if (!view?.el.isConnected || !isPaperView(view.paper)) continue; - if (view.paper.getCellViewPortalNode(view)) return true; - } - return false; - } public getPaperStore = (id: string) => { return this.paperStores.get(id); }; diff --git a/packages/joint-react/src/store/index.ts b/packages/joint-react/src/store/index.ts index 8e10a0eea4..7685b561fe 100644 --- a/packages/joint-react/src/store/index.ts +++ b/packages/joint-react/src/store/index.ts @@ -1,3 +1,2 @@ export * from './graph-store'; export * from './paper-store'; -export * from './create-elements-size-observer'; diff --git a/packages/joint-react/src/store/measurement.ts b/packages/joint-react/src/store/measurement.ts new file mode 100644 index 0000000000..f07a313a3a --- /dev/null +++ b/packages/joint-react/src/store/measurement.ts @@ -0,0 +1,373 @@ +import { mvc, type dia } from '@joint/core'; +import type { CellId, ElementLayout } from '../types/cell.types'; +import type { PaperStore } from './paper-store'; +import { simpleScheduler } from '../utils/scheduler'; +import { warnResizeOnAutoSizedElement } from '../utils/dev-warnings'; + +/** + * `dia.Cell.set()` option key used to mark writes that originate from the + * auto-size / measurement pipeline. `change:size` listeners can read it to + * distinguish measurement writes from external ones (controlled-mode sync, + * `cell.resize`, interactive tools) and avoid feedback loops. Exported for + * plugin authors and `@joint/react-plus` via `@joint/react/internal`. + */ +export const AUTO_SIZE_OPTION = 'autoSize'; + +/** + * Reference point that stays fixed when an auto-sized element's measured size + * changes. Mirrors CSS `transform-origin` semantics. + * + * - `'top-left'` (default): element grows right/down, top-left stays put. + * - `'center'`: element grows symmetrically, geometric center stays put. + * + * Only affects writes from the {@link useMeasureElement} pipeline. Manual `cell.resize()`, + * interactive resize tools, and direct `cell.set('size', ...)` calls are unaffected. + * @group Types + */ +export type AutoSizeOrigin = 'top-left' | 'center'; + +const OBSERVER_OPTIONS: ResizeObserverOptions = { box: 'border-box' }; +/** Tolerance that absorbs sub-pixel rendering jitter (notably on Safari). */ +const EPSILON = 0.5; + +/** A measured layout: `width`/`height` are required, `x`/`y` may be omitted. */ +type MeasuredLayout = Pick & + Partial>; + +/** + * The element's measurement, passed to a {@link TransformElementLayout} callback. + * Carries the element's current `x`, `y`, and `angle` together with the freshly + * measured `width` and `height`, plus the underlying model and cell id. + * @expand + * @group Types + */ +export interface TransformElementLayoutParams extends Required { + /** The JointJS `dia.Element` instance being measured. */ + readonly model: dia.Element; + /** Id of the cell being measured. */ + readonly id: CellId; +} + +/** + * Adjusts a measured element layout before it is written to the graph. Receives + * the element's current geometry plus its newly measured size, and returns the + * `width`/`height` (and optionally `x`/`y`) to apply — use it to clamp sizes, + * snap to a grid, or reposition while auto-sizing. Pass it via the `transform` + * option of {@link useMeasureElement}. + * @example + * ```tsx + * import type { TransformElementLayout } from '@joint/react'; + * + * // Never let a measured element shrink below 80px wide. + * const transform: TransformElementLayout = ({ width, height }) => ({ + * width: Math.max(width, 80), + * height, + * }); + * ``` + * @see {@link TransformElementLayoutParams} + * @group Types + */ +export type TransformElementLayout = (params: TransformElementLayoutParams) => MeasuredLayout; + +/** A DOM node whose size drives the size of the element `id`. */ +interface MeasuredNode { + readonly id: CellId; + readonly node: HTMLElement | SVGElement; + readonly transform?: TransformElementLayout; +} + +/** A registered {@link MeasuredNode} plus the last size it reported. */ +interface ObservedNode extends MeasuredNode { + lastWidth?: number; + lastHeight?: number; +} + +interface Options { + readonly graph: dia.Graph; + readonly autoSizeOrigin: AutoSizeOrigin; + /** The graph store's live paper registry: papers are what render and measure. */ + readonly paperStores: ReadonlyMap; +} + +const isSameSize = (a: number, b: number) => Math.abs(a - b) <= EPSILON; +const roundToTwoDecimals = (value: number) => Math.round(value * 100) / 100; + +/** + * Everything `@joint/react` knows about measuring elements, for one graph: + * sizing elements from their rendered DOM node ({@link useMeasureElement}) and + * telling when sizes are settled ({@link useOnElementsMeasured}). + * + * The sizing version changes once per settled change. An element is outstanding from + * arriving in the graph until it is accounted for: its React content committed + * without registering a node (`markRendered`), no paper renders it + * (`settleUnrendered`), the node it registered was measured, or it left the + * graph. A zero size is never read as "waiting": it is a legal final size. + * Sizes the application writes are not measurements and never bump. + * + * The `ResizeObserver` is created by the first `observe()`, so a diagram that + * never measures does not have one. + * @param options - the graph, the auto-size origin and the paper registry + * @returns the measurement controller owned by the graph store + */ +export function createMeasurement(options: Options) { + const { graph, autoSizeOrigin, paperStores } = options; + // `0` while nothing is measured; otherwise the latest of `lastVersion`, which + // only grows, so a reset followed by a pass never repeats a value. + let version = 0; + let lastVersion = 0; + const versionListeners = new Set<() => void>(); + const subscribe = (versionListener: () => void) => { + versionListeners.add(versionListener); + return () => { + versionListeners.delete(versionListener); + }; + }; + const setVersion = (next: number) => { + version = next; + for (const versionListener of versionListeners) versionListener(); + }; + let elementCount = 0; + const outstanding = new Set(); + let hasUndeliveredChange = false; + // Registered nodes per element: the last one is the observed one, and the + // previous one takes over again when it unmounts. + const nodesById = new Map(); + const observedByNode = new WeakMap(); + let resizeObserver: ResizeObserver | undefined; + + const deliver = () => { + if (!hasUndeliveredChange) return; + // Papers account for outstanding elements; without one nothing renders or + // measures, so there is nothing to wait for. + if (outstanding.size > 0 && paperStores.size > 0) return; + // An empty diagram has nothing measured. + if (elementCount === 0) { + if (version > 0) setVersion(0); + return; + } + hasUndeliveredChange = false; + lastVersion += 1; + setVersion(lastVersion); + }; + // One round later than the graph events that settle an element: the + // projection commits those events in the round they are queued in, and a + // subscriber of the new version reads the committed cells. + const deliverNextRound = () => simpleScheduler(deliver); + const scheduleDelivery = () => simpleScheduler(deliverNextRound); + const settle = (id: CellId) => { + if (outstanding.delete(id)) scheduleDelivery(); + }; + /** + * Settles an element that nothing measures any more: it is not waiting for a + * measurement either. Checked after this tick, because an effect that re-runs + * (StrictMode) registers the node again right away. + */ + const settleOnceUnobserved = (id: CellId) => + simpleScheduler(() => { + if (!nodesById.has(id)) settle(id); + }); + /** A change the subscribers are owed an event for, once nothing is outstanding. */ + const markChanged = () => { + hasUndeliveredChange = true; + scheduleDelivery(); + }; + const track = (element: dia.Element) => { + elementCount += 1; + outstanding.add(element.id); + markChanged(); + }; + + /** + * Whether some paper mounted the element's view and renders React content + * into it, the only way a node gets registered. + */ + const isRendered = (id: CellId) => { + for (const { renderElement, paper } of paperStores.values()) { + if (!renderElement) continue; + const view = paper.getElementView(id); + if (view?.el.isConnected && paper.getCellViewPortalNode(view)) return true; + } + return false; + }; + + const write = (model: dia.Element, layout: MeasuredLayout) => { + const { width, height, x, y } = layout; + const attributes: dia.Cell.Attributes = { size: { width, height } }; + if (x !== undefined && y !== undefined) { + attributes.position = { x, y }; + } else if (autoSizeOrigin === 'center') { + // Keep the geometric center fixed. With 'top-left' the position is not + // written: the top-left stays put and the element grows right/down. + const center = model.getCenter(); + attributes.position = { x: center.x - width / 2, y: center.y - height / 2 }; + } + model.set(attributes, { [AUTO_SIZE_OPTION]: true }); + }; + + /** + * The write a reported box asks for, or `undefined` when there is none: a + * hidden (`display: none`) or detached node reports zero, which is never + * written, and a box equal to the model's size or to the last report is a no-op. + */ + const toWrite = (observed: ObservedNode, box: ResizeObserverSize | undefined) => { + if (!box) return; + const width = roundToTwoDecimals(box.inlineSize); + const height = roundToTwoDecimals(box.blockSize); + if (width <= 0 || height <= 0) return; + const { id, transform, lastWidth = 0, lastHeight = 0 } = observed; + const model = graph.getCell(id); + if (!model?.isElement()) return; + const size = model.size(); + if (isSameSize(size.width, width) && isSameSize(size.height, height)) return; + if (isSameSize(lastWidth, width) && isSameSize(lastHeight, height)) return; + observed.lastWidth = width; + observed.lastHeight = height; + const { x, y } = model.position(); + const params = { x, y, angle: model.angle(), model, width, height, id }; + return [model, transform ? transform(params) : { width, height }] as const; + }; + + const onResize: ResizeObserverCallback = (entries) => { + const writes: Array = []; + for (const { target, borderBoxSize } of entries) { + const observed = observedByNode.get(target); + if (!observed) continue; + // Measured, whether or not a size is written: a measurement equal to the + // model's size (the application pre-sized it) writes nothing. + settle(observed.id); + const pending = toWrite(observed, borderBoxSize?.[0]); + if (pending) writes.push(pending); + } + if (writes.length === 0) return; + graph.startBatch('auto-size'); + for (const [model, layout] of writes) write(model, layout); + graph.stopBatch('auto-size'); + }; + + const startObserving = (observed: ObservedNode) => { + resizeObserver ??= new ResizeObserver(onResize); + resizeObserver.observe(observed.node, OBSERVER_OPTIONS); + observedByNode.set(observed.node, observed); + }; + const stopObserving = (observed: ObservedNode) => { + resizeObserver?.unobserve(observed.node); + observedByNode.delete(observed.node); + }; + + // An external graph may already hold elements. + for (const element of graph.getElements()) track(element); + + const listener = new mvc.Listener(); + listener.listenTo(graph, 'add', (cell: dia.Cell) => { + if (cell.isElement()) track(cell); + }); + listener.listenTo(graph, 'remove', (cell: dia.Cell) => { + if (!cell.isElement()) return; + // What is left has to be laid out again, so a removal is a change too. + elementCount -= 1; + outstanding.delete(cell.id); + markChanged(); + }); + listener.listenTo(graph, 'reset', (collection: mvc.Collection) => { + // The reset replaces the diagram: until its first settled pass nothing is measured. + elementCount = 0; + outstanding.clear(); + hasUndeliveredChange = false; + if (version > 0) setVersion(0); + // `reset` suppresses the per-cell `add` events. + for (const cell of collection.models) if (cell.isElement()) track(cell); + }); + listener.listenTo( + graph, + 'change:size', + (cell: dia.Cell, _size: dia.Size, changeOptions: dia.Cell.Options = {}) => { + if (changeOptions[AUTO_SIZE_OPTION]) { + outstanding.delete(cell.id); + markChanged(); + } else if (nodesById.has(cell.id)) { + // The measured content size overrides this resize. + warnResizeOnAutoSizedElement(cell.id); + } + } + ); + + return { + /** + * The sizing version: `0` while nothing is measured, otherwise a number + * that changes once per settled change. See {@link useOnElementsMeasured}. + */ + getSizingVersion: () => version, + /** + * Calls `versionListener` each time the sizing version changes. + * @param versionListener - called synchronously with the change + * @returns the unsubscribe function + */ + subscribe, + /** + * Sizes the element from `node`, for as long as the returned cleanup has not run. + * @param measured - the element id, its DOM node and an optional transform + * @returns cleanup that stops measuring the node + */ + observe(measured: MeasuredNode) { + const { id } = measured; + const observed: ObservedNode = { ...measured }; + const nodes = nodesById.get(id) ?? []; + const active = nodes.at(-1); + if (active) stopObserving(active); + else nodesById.set(id, nodes); + nodes.push(observed); + startObserving(observed); + return () => { + const registered = nodesById.get(id); + const index = registered?.indexOf(observed) ?? -1; + if (!registered || index === -1) return; + const wasObserved = index === registered.length - 1; + registered.splice(index, 1); + if (!wasObserved) return; + stopObserving(observed); + const previous = registered.at(-1); + if (previous) startObserving(previous); + else { + nodesById.delete(id); + settleOnceUnobserved(id); + } + }; + }, + /** + * An element's React content committed. Called after its subtree's layout + * effects, so an element that registered a node stays outstanding until + * measured, while one that nothing measures is settled now. + * @param id - the rendered element + */ + markRendered(id: CellId) { + if (!nodesById.has(id)) settle(id); + }, + /** + * A paper finished a render pass (`render:done`): every view it mounts for + * this pass is in the DOM now. An outstanding element that no paper renders + * (viewport culling, `cellVisibility`, a hidden group) is not going to be + * measured, so it is settled with the size it has. O(outstanding), which is + * empty between changes. + */ + settleUnrendered() { + for (const id of outstanding) { + if (!nodesById.has(id) && !isRendered(id)) settle(id); + } + }, + /** + * A paper was removed. Only papers hold outstanding elements back, so with + * the last one gone the event is due. Checked on the next pass, not now: a + * paper re-created right away (StrictMode, a hot reload) keeps them waiting. + */ + onPaperRemoved: scheduleDelivery, + /** Stops listening to the graph and observing nodes. */ + destroy() { + listener.stopListening(); + resizeObserver?.disconnect(); + nodesById.clear(); + }, + }; +} + +/** Controller returned by {@link createMeasurement}. */ +export type Measurement = ReturnType; diff --git a/packages/joint-react/src/store/paper-store.ts b/packages/joint-react/src/store/paper-store.ts index 9ddf68b5da..d6f6b2de4c 100644 --- a/packages/joint-react/src/store/paper-store.ts +++ b/packages/joint-react/src/store/paper-store.ts @@ -104,8 +104,8 @@ export class PaperStore { /** Link changes pending flush, populated by clearView, flushed in afterRender. */ private pendingLinkChanges: Map> = new Map(); - /** The graph store's `render:done` listener, kept to unsubscribe an adopted paper. */ - private readonly settleUnrenderedElements: () => void; + /** The measurement's `render:done` listener, kept to unsubscribe an adopted paper. */ + private readonly settleUnrendered: () => void; constructor(options: PaperStoreOptions) { const { @@ -180,10 +180,8 @@ export class PaperStore { // prop-update effect runs (or when the store is used without the hook). this.nativeCellVisibility = this.paper.options.cellVisibility; - // After each render pass the graph store learns which added elements no - // paper renders, so they do not hold `useOnElementsMeasured` open. - this.settleUnrenderedElements = graphStore.settleUnrenderedElements; - this.paper.on('render:done', this.settleUnrenderedElements); + this.settleUnrendered = graphStore.measurement.settleUnrendered; + this.paper.on('render:done', this.settleUnrendered); if (transform !== undefined) { this.paper.matrix(toSVGMatrix(transform)); @@ -273,7 +271,7 @@ export class PaperStore { * Should be called when the paper is being removed from the graph store. */ public destroy = () => { - this.paper.off('render:done', this.settleUnrenderedElements); + this.paper.off('render:done', this.settleUnrendered); // An adopted paper is owned by its creator (e.g. ``), which // removes it itself — removing it here would kill a paper still in use, // breaking the next adoption (notably under React StrictMode remounts). diff --git a/packages/joint-react/src/store/state-container.ts b/packages/joint-react/src/store/state-container.ts index b578a06fb2..b4d689041a 100644 --- a/packages/joint-react/src/store/state-container.ts +++ b/packages/joint-react/src/store/state-container.ts @@ -1,6 +1,6 @@ /* eslint-disable unicorn/prefer-spread */ import { simpleScheduler } from '../utils/scheduler'; -import { isStrictEqual } from '../utils/selector-utils'; +import { isStrictEqual } from '../selectors/selector-utils'; import type { CellId, AnyCellRecord } from '../types/cell.types'; /** diff --git a/packages/joint-react/src/utils/__tests__/scheduler.test.ts b/packages/joint-react/src/utils/__tests__/scheduler.test.ts index 29fe263ad3..041c7edc5c 100644 --- a/packages/joint-react/src/utils/__tests__/scheduler.test.ts +++ b/packages/joint-react/src/utils/__tests__/scheduler.test.ts @@ -95,4 +95,75 @@ describe('createScheduler', () => { expect(calls).toEqual(['batch-1', 'batch-2']); }); + // Regression: a callback that threw left the scheduler "flushing" forever, so + // every later callback, for every graph on the page, was silently dropped. + describe('a callback that throws', () => { + beforeEach(() => { + jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + it('does not stop the other callbacks of the flush, nor later flushes', async () => { + const scheduler = createScheduler(); + const calls: string[] = []; + scheduler(() => { + throw new Error('boom'); + }); + scheduler(() => calls.push('same flush')); + await Promise.resolve(); + expect(calls).toEqual(['same flush']); + + scheduler(() => calls.push('later flush')); + await Promise.resolve(); + expect(calls).toEqual(['same flush', 'later flush']); + }); + + it('still surfaces the error, on a later task', async () => { + const scheduler = createScheduler(); + scheduler(() => { + throw new Error('boom'); + }); + await Promise.resolve(); + + expect(() => jest.runOnlyPendingTimers()).toThrow('boom'); + }); + }); + + // Regression: callbacks that keep scheduling more work were drained in one + // endless microtask, which freezes the page. The flush now yields. + describe('callbacks that keep scheduling each other', () => { + beforeEach(() => { + jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + it('yields to the event loop and reports it, and carries on afterwards', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => {}); + const scheduler = createScheduler(); + let runs = 0; + const reschedule = () => { + runs += 1; + if (runs < 1000) scheduler(reschedule); + }; + scheduler(reschedule); + await Promise.resolve(); + + // One flush did a bounded amount of work, then handed control back. + expect(runs).toBeGreaterThan(1); + expect(runs).toBeLessThan(1000); + expect(error).toHaveBeenCalledWith(expect.stringContaining('keeps scheduling')); + + // The rest is drained over later tasks, not lost. + for (let task = 0; task < 100 && runs < 1000; task += 1) { + jest.runOnlyPendingTimers(); + await Promise.resolve(); + } + expect(runs).toBe(1000); + error.mockRestore(); + }); + }); }); diff --git a/packages/joint-react/src/utils/collection-subscription.ts b/packages/joint-react/src/utils/collection-subscription.ts deleted file mode 100644 index 14d267b3e5..0000000000 --- a/packages/joint-react/src/utils/collection-subscription.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { mvc, type dia } from '@joint/core'; -import type { AnyCellRecord, CellId } from '../types/cell.types'; -import type { ReadonlyContainer } from '../store/state-container'; - -/** - * Creates a self-managing subscription for the collection form of {@link useCells}. - * - * Subscribes to each cell ID in the container (per-ID granularity) AND to - * collection membership events (`add`/`remove`/`reset`). On membership - * change, resubscribes to the new set of IDs. - * - * All changes bump `versionRef` so `getSnapshot` returns a new value, - * causing `useSyncExternalStoreWithSelector` to re-evaluate `select`. - */ -export function subscribeToCollection( - collection: mvc.Collection, - container: ReadonlyContainer, - listener: () => void, - collectionIdsRef: { current: readonly CellId[] }, - versionRef: { current: number } -): () => void { - let cellUnsubs: Array<() => void> = []; - - const bumpAndNotify = () => { - versionRef.current++; - listener(); - }; - - const resubscribeCells = () => { - for (const unsub of cellUnsubs) unsub(); - const ids = collection.models.map((m) => m.id as CellId); - collectionIdsRef.current = ids; - cellUnsubs = ids.map((id) => container.subscribeById(id, bumpAndNotify)); - }; - - resubscribeCells(); - - const controller = new mvc.Listener(); - const onMembershipChange = () => { - resubscribeCells(); - bumpAndNotify(); - }; - controller.listenTo(collection, 'add remove reset', onMembershipChange); - - return () => { - for (const unsub of cellUnsubs) unsub(); - controller.stopListening(); - }; -} diff --git a/packages/joint-react/src/utils/dev-warnings.ts b/packages/joint-react/src/utils/dev-warnings.ts index 85ca3b68fa..692f260a2b 100644 --- a/packages/joint-react/src/utils/dev-warnings.ts +++ b/packages/joint-react/src/utils/dev-warnings.ts @@ -72,7 +72,7 @@ export function warnUnstableSelector( 'Options to fix:\n' + ' 1. Return primitives: (cells) => cells.map(c => c.id)\n' + ' 2. Return cell records directly: (cells) => cells.filter(predicate)\n' + - ' 3. Provide a custom isEqual: useCells(input, selector, shallowEqual)\n' + ' 3. Provide a custom isEqual as the last argument of the hook\n' ); } @@ -125,3 +125,40 @@ export function warnResizeOnAutoSizedElement(cellId: dia.Cell.ID): void { 'explicit size (e.g. from FreeTransform / Halo).' ); } + +/** + * Warns once when the graph state is read outside of the selector it was + * handed to (kept in a variable and read later, or after an `await`). The hook + * subscribes to what the selector reads while it runs, so such a read is not + * followed by any update. Dev-only, tree-shaken in production. + */ +export function warnGraphStateReadOutsideSelector(): void { + if (process.env.NODE_ENV === 'production') return; + const key = 'graph-state-read-outside-selector'; + if (WARNED.has(key)) return; + WARNED.add(key); + + console.warn( + '[useGraphState] The graph state was read outside of its selector. The hook follows only ' + + 'what the selector reads while it runs, so this read will not update.\n\n' + + 'Fix: read everything you need inside the selector and return it.' + ); +} + +/** + * Reports once that store callbacks keep scheduling each other, typically a + * change handler that changes what it listens to. The scheduler then spreads + * the work over later tasks instead of freezing the page. Dev-only, + * tree-shaken in production. + */ +export function warnSchedulerCascade(): void { + if (process.env.NODE_ENV === 'production') return; + const key = 'scheduler-cascade'; + if (WARNED.has(key)) return; + WARNED.add(key); + + console.error( + '[@joint/react] A store callback keeps scheduling more work each time it runs, ' + + 'for example a change handler that adds or removes cells. Guard it so it settles.' + ); +} diff --git a/packages/joint-react/src/utils/scheduler.ts b/packages/joint-react/src/utils/scheduler.ts index eb50c4cfe1..ed63804391 100644 --- a/packages/joint-react/src/utils/scheduler.ts +++ b/packages/joint-react/src/utils/scheduler.ts @@ -1,3 +1,19 @@ +import { warnSchedulerCascade } from './dev-warnings'; + +/** Rounds of cascading callbacks one flush drains before it yields to the event loop. */ +const MAX_CASCADE_ROUNDS = 100; + +/** + * Rethrows an error on a later task, so it still surfaces (to `window.onerror` + * and the console) while the caller carries on. + * @param error - The error a callback threw. + */ +export function rethrowLater(error: unknown): void { + setTimeout(() => { + throw error; + }); +} + /** * Creates a microtask-based scheduler that batches callbacks. * @@ -17,21 +33,34 @@ export function createScheduler(): (callback: () => void) => void { // Process cascading callbacks in the same flush so React sees // all store changes as a single batched update. - while (callbacks.size > 0) { + for (let round = 0; callbacks.size > 0 && round < MAX_CASCADE_ROUNDS; round += 1) { const pending = callbacks; callbacks = new Set(); for (const callback of pending) { - callback(); + // One scheduler serves every graph on the page: a callback that throws + // must not stop the others, nor leave the scheduler flushing forever. + try { + callback(); + } catch (error) { + rethrowLater(error); + } } } flushing = false; + if (callbacks.size === 0) return; + // Callbacks keep scheduling each other (a change handler that changes what + // it listens to). Draining them here would never return and freeze the + // page, so the rest runs on a later task. + scheduled = true; + setTimeout(flush); + warnSchedulerCascade(); }; return (callback: () => void): void => { callbacks.add(callback); - // If we're already inside a flush, the while-loop will pick up + // If we're already inside a flush, the flush loop will pick up // the new callback — no need to schedule another microtask. if (scheduled || flushing) { return;