Skip to content

Commit 10be65e

Browse files
TomasEngstanderen
TomasEng
and
standeren
authored
fix: Clean up useBpmnEditor (#14471)
Co-authored-by: andreastanderen <[email protected]>
1 parent 52bb520 commit 10be65e

File tree

4 files changed

+143
-102
lines changed

4 files changed

+143
-102
lines changed

‎frontend/packages/process-editor/src/contexts/BpmnContext.tsx

+8
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ export type BpmnContextProps = {
1212
appLibVersion: string;
1313
bpmnDetails: BpmnDetails;
1414
setBpmnDetails: React.Dispatch<React.SetStateAction<BpmnDetails>>;
15+
isInitialized: boolean;
16+
setIsInitialized: React.Dispatch<React.SetStateAction<boolean>>;
17+
initialBpmnXml: string;
1518
};
1619

1720
export const BpmnContext = createContext<Partial<BpmnContextProps>>(undefined);
@@ -27,6 +30,8 @@ export const BpmnContextProvider = ({
2730
appLibVersion,
2831
}: Partial<BpmnContextProviderProps>) => {
2932
const [bpmnDetails, setBpmnDetails] = useState<BpmnDetails>(null);
33+
const [isInitialized, setIsInitialized] = useState<boolean>(false);
34+
const [initialBpmnXml] = useState<string>(bpmnXml);
3035

3136
const isEditAllowed =
3237
supportsProcessEditor(appLibVersion) ||
@@ -56,6 +61,9 @@ export const BpmnContextProvider = ({
5661
appLibVersion,
5762
bpmnDetails,
5863
setBpmnDetails,
64+
isInitialized,
65+
setIsInitialized,
66+
initialBpmnXml,
5967
}}
6068
>
6169
{children}

‎frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx

+25-19
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { RenderHookResult } from '@testing-library/react';
33
import { renderHook, waitFor, act } from '@testing-library/react';
44
import type { UseBpmnEditorResult } from './useBpmnEditor';
55
import { useBpmnEditor } from './useBpmnEditor';
6-
import type { BpmnContextProviderProps } from '../contexts/BpmnContext';
6+
import type { BpmnContextProps, BpmnContextProviderProps } from '../contexts/BpmnContext';
77
import { BpmnContextProvider, useBpmnContext } from '../contexts/BpmnContext';
88
import type { BpmnApiContextProps } from '../contexts/BpmnApiContext';
99
import { BpmnApiContextProvider } from '../contexts/BpmnApiContext';
@@ -16,12 +16,11 @@ import { EventListeners } from '../../test/EventListeners';
1616
import type {
1717
BpmnBusinessObjectEditor,
1818
BpmnExtensionElementsEditor,
19-
} from '@altinn/process-editor/types/BpmnBusinessObjectEditor';
19+
} from '../types/BpmnBusinessObjectEditor';
2020
import { BpmnTypeEnum } from '../enum/BpmnTypeEnum';
2121
import type { BpmnTaskType } from '../types/BpmnTaskType';
22-
import type { OnProcessTaskEvent } from '@altinn/process-editor/types/OnProcessTask';
23-
import type { BpmnDetails } from '@altinn/process-editor/types/BpmnDetails';
24-
import type { SelectionChangedEvent } from '@altinn/process-editor/types/SelectionChangeEvent';
22+
import type { OnProcessTaskEvent } from '../types/OnProcessTask';
23+
import type { SelectionChangedEvent } from '../types/SelectionChangeEvent';
2524
import type BpmnModeler from 'bpmn-js/lib/Modeler';
2625

2726
// Test data:
@@ -170,23 +169,22 @@ describe('useBpmnEditor', () => {
170169
oldSelection: [],
171170
newSelection: [element],
172171
};
173-
const { result } = await setupWithBpmnDetails();
172+
const { result } = await setupWithBpmnContext();
174173
act(() => eventListeners.triggerEvent('selection.changed', selectionChangedEvent));
175-
expect(result.current.bpmnDetails.element).toEqual(element);
174+
expect(result.current.bpmnContext.bpmnDetails.element).toEqual(element);
176175
});
177176

178177
it('Updates BPMN details with null when "selection.changed" event is triggered with no new selected object', async () => {
179178
const selectionChangedEvent: SelectionChangedEvent = {
180179
oldSelection: [element],
181180
newSelection: [],
182181
};
183-
const { result } = await setupWithBpmnDetails();
182+
const { result } = await setupWithBpmnContext();
184183
act(() => eventListeners.triggerEvent('selection.changed', selectionChangedEvent));
185-
expect(result.current.bpmnDetails).toBe(null);
184+
expect(result.current.bpmnContext.bpmnDetails).toBe(null);
186185
});
187186

188-
// Todo: Remove skip when this test passes. Fixing this will resolve https://github.com/Altinn/altinn-studio/issues/13035.
189-
it.skip('Calls only the most recent saveBpmn function when the "commandStack.changed" event is triggered', async () => {
187+
it('Calls only the most recent saveBpmn function when the "commandStack.changed" event is triggered', async () => {
190188
const saveBpmn1 = jest.fn();
191189
const saveBpmn2 = jest.fn();
192190
const bpmnApiContextProps: Partial<BpmnApiContextProps> = {
@@ -202,6 +200,14 @@ describe('useBpmnEditor', () => {
202200
expect(saveBpmn1).not.toHaveBeenCalled();
203201
expect(saveBpmn2).toHaveBeenCalledTimes(1);
204202
});
203+
204+
it('Resets the modeler ref when the callback is called with null', async () => {
205+
const { result } = await setupWithBpmnContext();
206+
const { modelerRef } = result.current.bpmnContext;
207+
expect(modelerRef.current).not.toBeNull();
208+
act(() => result.current.bpmnEditor(null));
209+
expect(modelerRef.current).toBeNull();
210+
});
205211
});
206212

207213
type BpmnProviderProps = {
@@ -243,25 +249,25 @@ function renderWithBpmnProviders(
243249
);
244250
}
245251

246-
async function setupWithBpmnDetails(): Promise<
247-
RenderHookResult<UseBpmnEditorAndDetailsResult, void>
252+
async function setupWithBpmnContext(): Promise<
253+
RenderHookResult<UseBpmnEditorAndContextResult, void>
248254
> {
249255
const wrapper = ({ children }) => renderWithBpmnProviders(children);
250-
const utils = renderHook(() => useBpmnEditorAndDetails(), { wrapper });
256+
const utils = renderHook(() => useBpmnEditorAndContext(), { wrapper });
251257
const { result } = utils;
252258
const div = document.createElement('div');
253259
result.current.bpmnEditor(div);
254260
await waitFor(expect(getModeler).toHaveBeenCalled);
255261
return utils;
256262
}
257263

258-
type UseBpmnEditorAndDetailsResult = {
264+
type UseBpmnEditorAndContextResult = {
259265
bpmnEditor: UseBpmnEditorResult;
260-
bpmnDetails: BpmnDetails;
266+
bpmnContext: Partial<BpmnContextProps>;
261267
};
262268

263-
const useBpmnEditorAndDetails = (): UseBpmnEditorAndDetailsResult => {
269+
const useBpmnEditorAndContext = (): UseBpmnEditorAndContextResult => {
264270
const bpmnEditor = useBpmnEditor();
265-
const { bpmnDetails } = useBpmnContext();
266-
return { bpmnEditor, bpmnDetails };
271+
const bpmnContext = useBpmnContext();
272+
return { bpmnEditor, bpmnContext };
267273
};

‎frontend/packages/process-editor/src/hooks/useBpmnEditor.ts

+107-83
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useCallback } from 'react';
1+
import { useCallback, useEffect } from 'react';
22
import { useBpmnContext } from '../contexts/BpmnContext';
33
import { BpmnModelerInstance } from '../utils/bpmnModeler/BpmnModelerInstance';
44
import { useBpmnConfigPanelFormContext } from '../contexts/BpmnConfigPanelContext';
@@ -7,53 +7,56 @@ import type { TaskEvent } from '../types/TaskEvent';
77
import type { SelectionChangedEvent } from '../types/SelectionChangeEvent';
88
import { getBpmnEditorDetailsFromBusinessObject } from '../utils/bpmnObjectBuilders';
99
import { useStudioRecommendedNextActionContext } from '@studio/components';
10+
import type Modeler from 'bpmn-js/lib/Modeler';
1011

1112
// Wrapper around bpmn-js to Reactify it
1213

1314
export type UseBpmnEditorResult = (div: HTMLDivElement) => void;
1415

1516
export const useBpmnEditor = (): UseBpmnEditorResult => {
16-
const { getUpdatedXml, bpmnXml, modelerRef, setBpmnDetails } = useBpmnContext();
17+
const { getUpdatedXml, setBpmnDetails } = useBpmnContext();
1718
const { metadataFormRef, resetForm } = useBpmnConfigPanelFormContext();
1819
const { addAction } = useStudioRecommendedNextActionContext();
1920

2021
const { saveBpmn, onProcessTaskAdd, onProcessTaskRemove } = useBpmnApiContext();
2122

22-
const handleCommandStackChanged = async () => {
23-
saveBpmn(await getUpdatedXml(), metadataFormRef.current || null);
23+
const handleCommandStackChanged = useCallback(async () => {
24+
const xml = await getUpdatedXml();
25+
saveBpmn(xml, metadataFormRef.current || null);
2426
resetForm();
25-
};
26-
27-
const handleShapeAdd = (taskEvent: TaskEvent): void => {
28-
const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(taskEvent?.element?.businessObject);
29-
onProcessTaskAdd({
30-
taskEvent,
31-
taskType: bpmnDetails.taskType,
32-
});
33-
if (
34-
bpmnDetails.taskType === 'data' ||
35-
bpmnDetails.taskType === 'payment' ||
36-
bpmnDetails.taskType === 'signing'
37-
)
38-
addAction(bpmnDetails.id);
39-
};
40-
41-
const handleShapeRemove = (taskEvent: TaskEvent): void => {
42-
const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(taskEvent?.element?.businessObject);
43-
onProcessTaskRemove({
44-
taskEvent,
45-
taskType: bpmnDetails.taskType,
46-
});
47-
};
48-
49-
const handleSelectionChange = (selectionEvent: SelectionChangedEvent): void => {
50-
if (selectionEvent.newSelection.length !== 1) {
51-
setBpmnDetails(null);
52-
return;
53-
}
54-
const selectedElement = selectionEvent.newSelection[0];
55-
updateBpmnDetails(selectedElement);
56-
};
27+
}, [saveBpmn, resetForm, metadataFormRef, getUpdatedXml]);
28+
29+
const handleShapeAdd = useCallback(
30+
async (taskEvent: TaskEvent): Promise<void> => {
31+
const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(
32+
taskEvent?.element?.businessObject,
33+
);
34+
onProcessTaskAdd({
35+
taskEvent,
36+
taskType: bpmnDetails.taskType,
37+
});
38+
if (
39+
bpmnDetails.taskType === 'data' ||
40+
bpmnDetails.taskType === 'payment' ||
41+
bpmnDetails.taskType === 'signing'
42+
)
43+
addAction(bpmnDetails.id);
44+
},
45+
[addAction, onProcessTaskAdd],
46+
);
47+
48+
const handleShapeRemove = useCallback(
49+
(taskEvent: TaskEvent): void => {
50+
const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(
51+
taskEvent?.element?.businessObject,
52+
);
53+
onProcessTaskRemove({
54+
taskEvent,
55+
taskType: bpmnDetails.taskType,
56+
});
57+
},
58+
[onProcessTaskRemove],
59+
);
5760

5861
const updateBpmnDetails = useCallback(
5962
(element: any) => {
@@ -66,52 +69,73 @@ export const useBpmnEditor = (): UseBpmnEditorResult => {
6669
[setBpmnDetails],
6770
);
6871

69-
const initializeEditor = async () => {
70-
if (!modelerRef.current) return;
71-
try {
72-
await modelerRef.current.importXML(bpmnXml);
73-
const canvas: any = modelerRef.current.get('canvas');
74-
canvas.zoom('fit-viewport');
75-
} catch (exception) {
76-
console.log('An error occurred while rendering the viewer:', exception);
77-
}
78-
};
79-
80-
const initializeBpmnChanges = () => {
81-
modelerRef.current.on('commandStack.changed', async (): Promise<void> => {
82-
await handleCommandStackChanged();
83-
});
84-
modelerRef.current.on('shape.added', (taskEvent: TaskEvent): void => {
85-
handleShapeAdd(taskEvent);
86-
});
87-
modelerRef.current.on('shape.remove', (taskEvent: TaskEvent): void => {
88-
handleShapeRemove(taskEvent);
89-
});
90-
modelerRef.current.on('selection.changed', (selectionEvent: SelectionChangedEvent): void => {
91-
handleSelectionChange(selectionEvent);
92-
});
93-
};
94-
95-
const canvasRef = useCallback((div: HTMLDivElement) => {
96-
if (modelerRef.current) return;
97-
98-
modelerRef.current = BpmnModelerInstance.getInstance(div);
99-
100-
initializeEditor().then(() => {
101-
// Wait for the initializeEditor to be initialized before attaching event listeners, to avoid trigger add.shape events on first draw
102-
initializeBpmnChanges();
103-
});
104-
105-
// eslint-disable-next-line react-hooks/exhaustive-deps
106-
}, []); // Missing dependencies are not added to avoid getModeler to be called multiple times
72+
const handleSelectionChange = useCallback(
73+
(selectionEvent: SelectionChangedEvent): void => {
74+
if (selectionEvent.newSelection.length !== 1) {
75+
setBpmnDetails(null);
76+
return;
77+
}
78+
const selectedElement = selectionEvent.newSelection[0];
79+
updateBpmnDetails(selectedElement);
80+
},
81+
[setBpmnDetails, updateBpmnDetails],
82+
);
10783

108-
useEffect(() => {
109-
// Destroy the modeler instance when the component is unmounted
110-
return () => {
111-
BpmnModelerInstance.destroyInstance();
112-
};
113-
// eslint-disable-next-line react-hooks/exhaustive-deps
114-
}, []);
115-
116-
return canvasRef;
84+
useModelerEventListener<void>('commandStack.changed', handleCommandStackChanged);
85+
useModelerEventListener<TaskEvent>('shape.added', handleShapeAdd);
86+
useModelerEventListener<TaskEvent>('shape.remove', handleShapeRemove);
87+
useModelerEventListener<SelectionChangedEvent>('selection.changed', handleSelectionChange);
88+
89+
return useEditorCallback();
11790
};
91+
92+
function useModelerEventListener<Event>(eventName: string, callback: (event: Event) => void): void {
93+
const { modelerRef, isInitialized } = useBpmnContext();
94+
95+
useEffect(() => {
96+
if (isInitialized) {
97+
const modeler = modelerRef.current;
98+
modeler.on(eventName, callback);
99+
return () => modeler.off(eventName, callback);
100+
}
101+
}, [isInitialized, eventName, callback, modelerRef]);
102+
}
103+
104+
function useEditorCallback(): (div: HTMLDivElement) => void {
105+
const { initialBpmnXml, modelerRef, setIsInitialized } = useBpmnContext();
106+
107+
const initialize = useCallback(
108+
(div: HTMLDivElement) => {
109+
const modeler = BpmnModelerInstance.getInstance(div);
110+
if (!modelerRef.current) {
111+
modelerRef.current = modeler;
112+
initializeEditor(modeler, initialBpmnXml).then(() => setIsInitialized(true));
113+
}
114+
},
115+
[setIsInitialized, modelerRef, initialBpmnXml],
116+
);
117+
118+
const cleanUp = useCallback(() => {
119+
setIsInitialized(false);
120+
modelerRef.current = null;
121+
BpmnModelerInstance.destroyInstance();
122+
}, [setIsInitialized, modelerRef]);
123+
124+
return useCallback(
125+
(div: HTMLDivElement) => {
126+
initialize(div);
127+
if (div === null) cleanUp(); // Todo: Change this to a return function when we have upgraded to React 19. https://react.dev/reference/react-dom/components/common#react-19-added-cleanup-functions-for-ref-callbacks
128+
},
129+
[initialize, cleanUp],
130+
);
131+
}
132+
133+
async function initializeEditor(modeler: Modeler, bpmnXml: string): Promise<void> {
134+
try {
135+
await modeler.importXML(bpmnXml);
136+
const canvas = modeler.get<{ zoom: (string: string) => void }>('canvas');
137+
canvas.zoom('fit-viewport');
138+
} catch (exception) {
139+
console.log('An error occurred while rendering the viewer:', exception);
140+
}
141+
}

‎frontend/packages/process-editor/test/mocks/bpmnContextMock.ts

+3
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@ const mockAppLibVersion8: string = '8.0.3';
99

1010
export const mockBpmnContextValue: BpmnContextProps = {
1111
bpmnXml: mockBPMNXML,
12+
initialBpmnXml: mockBPMNXML,
1213
appLibVersion: mockAppLibVersion8,
1314
getUpdatedXml: jest.fn(),
1415
isEditAllowed: true,
1516
bpmnDetails: mockBpmnDetails,
1617
setBpmnDetails: jest.fn(),
1718
modelerRef: mockModelerRef as any,
19+
isInitialized: true,
20+
setIsInitialized: jest.fn(),
1821
};
1922

2023
export const mockLayoutSets: LayoutSets = {

0 commit comments

Comments
 (0)