refactor: Make useBpmnEditor return callback ref

Author: TomasEng

frontend/packages/process-editor/src/components/Canvas/BPMNEditor/BPMNEditor.test.tsx

@@ -6,9 +6,7 @@ import { BpmnConfigPanelFormContextProvider } from '../../../contexts/BpmnConfig
 import { textMock } from '@studio/testing/mocks/i18nMock';
 
 jest.mock('../../../hooks/useBpmnEditor', () => ({
-  useBpmnEditor: jest.fn().mockReturnValue({
-    canvasRef: { current: document.createElement('div') },
-  }),
+  useBpmnEditor: jest.fn().mockReturnValue(jest.fn()),
 }));
 
 describe('BPMNEditor', () => {

frontend/packages/process-editor/src/components/Canvas/BPMNEditor/BPMNEditor.tsx

@@ -4,7 +4,7 @@ import { useBpmnEditor } from '../../../hooks/useBpmnEditor';
 import './BPMNEditor.css';
 
 export const BPMNEditor = (): React.ReactElement => {
-  const { canvasRef } = useBpmnEditor();
+  const canvasRef = useBpmnEditor();
 
-  return <div className={classes.editorContainer} ref={canvasRef}></div>;
+  return <div className={classes.editorContainer} ref={canvasRef} />;
 };

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

@@ -1,13 +1,12 @@
 import React from 'react';
 import { renderHook, waitFor } from '@testing-library/react';
 import { useBpmnEditor } from './useBpmnEditor';
-import { BpmnContextProvider, useBpmnContext } from '../contexts/BpmnContext';
+import { BpmnContextProvider } from '../contexts/BpmnContext';
 import { BpmnApiContextProvider } from '../contexts/BpmnApiContext';
 import { useBpmnModeler } from './useBpmnModeler';
 import type { BpmnDetails } from '../types/BpmnDetails';
 import type { LayoutSets } from 'app-shared/types/api/LayoutSetsResponse';
 import { getMockBpmnElementForTask, mockBpmnDetails } from '../../test/mocks/bpmnDetailsMock';
-import { mockModelerRef } from '../../test/mocks/bpmnModelerMock';
 import { getBpmnEditorDetailsFromBusinessObject } from '../utils/bpmnObjectBuilders';
 import { StudioRecommendedNextActionContextProvider } from '@studio/components';
 
@@ -64,7 +63,7 @@ jest.mock('../contexts/BpmnContext', () => ({
   useBpmnContext: jest.fn(() => ({
     getUpdatedXml: jest.fn(),
     modelerRef: { current: null },
-    setBpmnDetails: jest.fn(),
+    setBpmnDetails: setBpmnDetailsMock,
   })),
 }));
 
@@ -76,16 +75,6 @@ const setBpmnDetailsMock = jest.fn();
 const onProcessTaskAddMock = jest.fn();
 const onProcessTaskRemoveMock = jest.fn();
 
-const overrideUseBpmnContext = () => {
-  (useBpmnContext as jest.Mock).mockReturnValue({
-    getUpdatedXml: jest.fn(),
-    modelerRef: {
-      ...mockModelerRef,
-    },
-    setBpmnDetails: setBpmnDetailsMock,
-  });
-};
-
 const overrideUseBpmnModeler = (currentEventName: string, currentEvent: any) => {
   (useBpmnModeler as jest.Mock).mockReturnValue({
     getModeler: () => new BpmnModelerMockImpl(currentEventName, currentEvent),
@@ -124,29 +113,29 @@ describe('useBpmnEditor', () => {
   it('should call saveBpmn when "commandStack.changed" event is triggered on modelerInstance', async () => {
     const currentEventName = 'commandStack.changed';
     const currentEvent = { element: getMockBpmnElementForTask('data') };
-    renderUseBpmnEditor(false, currentEventName, currentEvent);
+    setup(currentEventName, currentEvent);
 
     await waitFor(() => expect(saveBpmnMock).toHaveBeenCalledTimes(1));
   });
 
   it('should handle "shape.added" event', async () => {
     const currentEvent = { element: getMockBpmnElementForTask('data') };
-    renderUseBpmnEditor(false, 'shape.added', currentEvent);
+    setup('shape.added', currentEvent);
 
     await waitFor(() => expect(onProcessTaskAddMock).toHaveBeenCalledTimes(1));
   });
 
   it('should handle "shape.remove" event', async () => {
     const currentEvent = { element: getMockBpmnElementForTask('data') };
-    renderUseBpmnEditor(false, 'shape.remove', currentEvent);
+    setup('shape.remove', currentEvent);
 
     await waitFor(() => expect(onProcessTaskRemoveMock).toHaveBeenCalledTimes(1));
   });
 
   it('should call setBpmnDetails with selected object when "selection.changed" event is triggered with new selection', async () => {
     const currentEventName = 'selection.changed';
     const currentEvent = { newSelection: [getMockBpmnElementForTask('data')], oldSelection: [] };
-    renderUseBpmnEditor(true, currentEventName, currentEvent);
+    setup(currentEventName, currentEvent);
 
     await waitFor(() => expect(setBpmnDetailsMock).toHaveBeenCalledTimes(1));
     expect(setBpmnDetailsMock).toHaveBeenCalledWith(expect.objectContaining(mockBpmnDetails));
@@ -155,20 +144,24 @@ describe('useBpmnEditor', () => {
   it('should call setBpmnDetails with null when "selection.changed" event is triggered with no new selected object', async () => {
     const currentEventName = 'selection.changed';
     const currentEvent = { oldSelection: [getMockBpmnElementForTask('data')], newSelection: [] };
-    renderUseBpmnEditor(true, currentEventName, currentEvent);
+    setup(currentEventName, currentEvent);
 
     await waitFor(() => expect(setBpmnDetailsMock).toHaveBeenCalledTimes(1));
     expect(setBpmnDetailsMock).toHaveBeenCalledWith(null);
   });
 });
 
+function setup(...params: Parameters<typeof renderUseBpmnEditor>): void {
+  const div = document.createElement('div');
+  const { result } = renderUseBpmnEditor(...params);
+  result.current(div);
+}
+
 const renderUseBpmnEditor = (
-  overrideBpmnContext: boolean,
   currentEventName: string,
   currentEvent: any,
   bpmnDetails = mockBpmnDetails,
 ) => {
-  overrideBpmnContext && overrideUseBpmnContext();
   overrideGetBpmnEditorDetailsFromBusinessObject(bpmnDetails);
   overrideUseBpmnModeler(currentEventName, currentEvent);
   return renderHook(() => useBpmnEditor(), { wrapper });

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

@@ -1,5 +1,4 @@
-import { type MutableRefObject, useEffect, useCallback, useRef } from 'react';
-import type BpmnModeler from 'bpmn-js/lib/Modeler';
+import { useEffect, useCallback } from 'react';
 import { useBpmnContext } from '../contexts/BpmnContext';
 import { useBpmnModeler } from './useBpmnModeler';
 import { useBpmnConfigPanelFormContext } from '../contexts/BpmnConfigPanelContext';
@@ -11,14 +10,10 @@ import { useStudioRecommendedNextActionContext } from '@studio/components';
 
 // Wrapper around bpmn-js to Reactify it
 
-type UseBpmnViewerResult = {
-  canvasRef: MutableRefObject<HTMLDivElement>;
-  modelerRef: MutableRefObject<BpmnModeler>;
-};
+type UseBpmnEditorResult = (div: HTMLDivElement) => void;
 
-export const useBpmnEditor = (): UseBpmnViewerResult => {
+export const useBpmnEditor = (): UseBpmnEditorResult => {
   const { getUpdatedXml, bpmnXml, modelerRef, setBpmnDetails } = useBpmnContext();
-  const canvasRef = useRef<HTMLDivElement | null>(null);
   const { metadataFormRef, resetForm } = useBpmnConfigPanelFormContext();
   const { getModeler, destroyModeler } = useBpmnModeler();
   const { addAction } = useStudioRecommendedNextActionContext();
@@ -98,14 +93,13 @@ export const useBpmnEditor = (): UseBpmnViewerResult => {
     });
   };
 
-  useEffect(() => {
-    if (!canvasRef.current) {
-      console.log('Canvas reference is not yet available in the DOM.');
-    }
+  const canvasRef = useCallback((div: HTMLDivElement) => {
+    if (modelerRef.current) return;
+
     // GetModeler can only be fetched from this hook once since the modeler creates a
     // new instance and will attach the same canvasRef container to all instances it fetches.
     // Set modelerRef.current to the Context so that it can be used in other components
-    modelerRef.current = getModeler(canvasRef.current);
+    modelerRef.current = getModeler(div);
 
     initializeEditor().then(() => {
       // Wait for the initializeEditor to be initialized before attaching event listeners, to avoid trigger add.shape events on first draw
@@ -123,5 +117,5 @@ export const useBpmnEditor = (): UseBpmnViewerResult => {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
-  return { canvasRef, modelerRef };
+  return canvasRef;
 };