From a52f6d9b948d9f06a4489b84a06f6985a0030d7f Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 26 Mar 2025 01:14:57 +0000 Subject: [PATCH] fix: prevent editor from loading twice before initialization (#1761) Data from previous editor instance was being processed by current editor instance and sometimes failed due to mismatch. For example, editing text editor or any other basic editor after opening an advanced problem like drag-n-drop crashed. Now the editor is only rendered after the initialization process is complete. --- src/editors/Editor.tsx | 7 ++++++- src/editors/VideoSelector.jsx | 6 +++++- src/editors/VideoSelector.test.jsx | 4 ++-- src/editors/hooks.test.jsx | 3 ++- src/editors/hooks.ts | 16 ++++++++++------ 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/editors/Editor.tsx b/src/editors/Editor.tsx index 33f24c9f3..9f2991607 100644 --- a/src/editors/Editor.tsx +++ b/src/editors/Editor.tsx @@ -29,7 +29,7 @@ const Editor: React.FC = ({ returnFunction = null, }) => { const dispatch = useDispatch(); - hooks.initializeApp({ + const loading = hooks.useInitializeApp({ dispatch, data: { blockId, @@ -43,6 +43,11 @@ const Editor: React.FC = ({ const EditorComponent = supportedEditors[blockType]; + // Do not load editor until everything is initialized. + if (loading) { + return null; + } + if (EditorComponent === undefined && blockId) { return ( { const dispatch = useDispatch(); - hooks.initializeApp({ + const loading = hooks.useInitializeApp({ dispatch, data: { blockId, @@ -21,6 +21,10 @@ const VideoSelector = ({ studioEndpointUrl, }, }); + // istanbul ignore if + if (loading) { + return null; + } return ( ); diff --git a/src/editors/VideoSelector.test.jsx b/src/editors/VideoSelector.test.jsx index f3d0e60ae..98423fc54 100644 --- a/src/editors/VideoSelector.test.jsx +++ b/src/editors/VideoSelector.test.jsx @@ -6,7 +6,7 @@ import * as hooks from './hooks'; import VideoSelector from './VideoSelector'; jest.mock('./hooks', () => ({ - initializeApp: jest.fn(), + useInitializeApp: jest.fn(), })); jest.mock('./containers/VideoGallery', () => 'VideoGallery'); @@ -32,7 +32,7 @@ describe('Video Selector', () => { describe('behavior', () => { it('calls initializeApp hook with dispatch, and passed data', () => { shallow(); - expect(hooks.initializeApp).toHaveBeenCalledWith({ + expect(hooks.useInitializeApp).toHaveBeenCalledWith({ dispatch: useDispatch(), data: initData, }); diff --git a/src/editors/hooks.test.jsx b/src/editors/hooks.test.jsx index d1a2f1120..ad6dc8d31 100644 --- a/src/editors/hooks.test.jsx +++ b/src/editors/hooks.test.jsx @@ -12,6 +12,7 @@ jest.mock('react', () => ({ ...jest.requireActual('react'), useRef: jest.fn(val => ({ current: val })), useEffect: jest.fn(), + useState: jest.fn(() => [false, jest.fn()]), useCallback: (cb, prereqs) => ({ cb, prereqs }), })); @@ -56,7 +57,7 @@ describe('hooks', () => { studioEndpointUrl: 'studioEndpointUrl', learningContextId: 'learningContextId', }; - hooks.initializeApp({ dispatch, data: fakeData }); + hooks.useInitializeApp({ dispatch, data: fakeData }); expect(dispatch).not.toHaveBeenCalledWith(fakeData); const [cb, prereqs] = useEffect.mock.calls[0]; expect(prereqs).toStrictEqual([ diff --git a/src/editors/hooks.ts b/src/editors/hooks.ts index 887467534..5669dd652 100644 --- a/src/editors/hooks.ts +++ b/src/editors/hooks.ts @@ -1,4 +1,4 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import analyticsEvt from './data/constants/analyticsEvt'; @@ -6,11 +6,15 @@ import analyticsEvt from './data/constants/analyticsEvt'; import { actions, thunkActions } from './data/redux'; import { RequestKeys } from './data/constants/requests'; -// eslint-disable-next-line react-hooks/rules-of-hooks -export const initializeApp = ({ dispatch, data }) => useEffect( - () => dispatch(thunkActions.app.initialize(data)), - [data?.blockId, data?.studioEndpointUrl, data?.learningContextId], -); +export const useInitializeApp = ({ dispatch, data }) => { + const [loading, setLoading] = useState(true); + useEffect(() => { + setLoading(true); + dispatch(thunkActions.app.initialize(data)); + setLoading(false); + }, [data?.blockId, data?.studioEndpointUrl, data?.learningContextId]); + return loading; +}; export const navigateTo = (destination: string | URL) => { window.location.assign(destination);