Skip to content

Commit

Permalink
Merge pull request #3027 from epam/2912-fix-memory-leak-for-ketcher-w…
Browse files Browse the repository at this point in the history
…hen-it-is-opened-inside-modal

#2912 - fix memory leak for ketcher when it is opened inside modal
  • Loading branch information
Zhirnoff authored Aug 9, 2023
2 parents 6856ccb + c41909c commit 48d89cd
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 7 deletions.
11 changes: 10 additions & 1 deletion packages/ketcher-react/src/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ import {
KETCHER_INIT_EVENT_NAME,
KETCHER_ROOT_NODE_CLASS_NAME,
} from './constants';
import { createRoot } from 'react-dom/client';

const mediaSizes = {
smallWidth: 1040,
smallHeight: 600,
};

interface EditorProps extends Omit<Config, 'element'> {
interface EditorProps extends Omit<Config, 'element' | 'appRoot'> {
onInit?: (ketcher: Ketcher) => void;
}

Expand All @@ -52,15 +53,23 @@ function Editor(props: EditorProps) {
const ketcherInitEvent = new Event(KETCHER_INIT_EVENT_NAME);

useEffect(() => {
const appRoot = createRoot(rootElRef.current as HTMLDivElement);
init({
...props,
element: rootElRef.current,
appRoot,
}).then((ketcher: Ketcher) => {
if (typeof onInit === 'function') {
onInit(ketcher);
window.dispatchEvent(ketcherInitEvent);
}
});
return () => {
// setTimeout is used to disable the warn msg from react "Attempted to synchronously unmount a root while React was already rendering"
setTimeout(() => {
appRoot.unmount();
});
};
// TODO: provide the list of dependencies after implementing unsubscribe function
}, []);

Expand Down
4 changes: 1 addition & 3 deletions packages/ketcher-react/src/hooks/useSubscribtionOnEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ export const useSubscriptionOnEvents = () => {
unsubscribe(getKetcherInstance());
};

window.addEventListener(KETCHER_INIT_EVENT_NAME, () => {
subscribeOnInit();
});
window.addEventListener(KETCHER_INIT_EVENT_NAME, subscribeOnInit);
return () => {
unsubscribeOnUnMount();
window.removeEventListener(KETCHER_INIT_EVENT_NAME, subscribeOnInit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { ButtonsConfig } from './ButtonsConfig';
import { Editor } from '../../editor';
import createApi from '../../api';
import { initApp } from '../../ui';
import { Root } from 'react-dom/client';

class KetcherBuilder {
private structService: StructService | null;
Expand Down Expand Up @@ -54,6 +55,7 @@ class KetcherBuilder {

async appendUiAsync(
element: HTMLDivElement | null,
appRoot: Root,
staticResourcesUrl: string,
errorHandler: (message: string) => void,
buttons?: ButtonsConfig,
Expand All @@ -63,6 +65,7 @@ class KetcherBuilder {
const editor = await new Promise<Editor>((resolve) => {
initApp(
element,
appRoot,
staticResourcesUrl,
{
buttons: buttons || {},
Expand Down
4 changes: 4 additions & 0 deletions packages/ketcher-react/src/script/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
* limitations under the License.
***************************************************************************/

import { Root } from 'react-dom/client';
import { ButtonsConfig, KetcherBuilder } from './builders';

import { StructServiceProvider } from 'ketcher-core';

interface Config {
element: HTMLDivElement | null;
appRoot: Root;
staticResourcesUrl: string;
structServiceProvider: StructServiceProvider;
buttons?: ButtonsConfig;
Expand All @@ -28,6 +30,7 @@ interface Config {

async function buildKetcherAsync({
element,
appRoot,
staticResourcesUrl,
structServiceProvider,
buttons,
Expand All @@ -39,6 +42,7 @@ async function buildKetcherAsync({
builder.appendServiceMode(structServiceProvider.mode);
await builder.appendUiAsync(
element,
appRoot,
staticResourcesUrl,
errorHandler,
buttons,
Expand Down
6 changes: 3 additions & 3 deletions packages/ketcher-react/src/script/ui/App/initApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ import { Ketcher, StructService } from 'ketcher-core';

import App from './App.container';
import { Provider } from 'react-redux';
import { createRoot } from 'react-dom/client';
import { Root } from 'react-dom/client';
import createStore from '../state';
import { initKeydownListener } from '../state/hotkeys';
import { initResize } from '../state/toolbar';
import { initMouseListener } from '../state/mouse';

function initApp(
element: HTMLDivElement | null,
appRoot: Root,
staticResourcesUrl: string,
options: any,
server: StructService,
Expand All @@ -41,8 +42,7 @@ function initApp(
store.dispatch(initMouseListener(element));
store.dispatch(initResize());

const root = createRoot(element!);
root.render(
appRoot.render(
<Provider store={store}>
<SettingsContext.Provider value={{ staticResourcesUrl }}>
<ErrorsContext.Provider value={{ errorHandler: options.errorHandler }}>
Expand Down

0 comments on commit 48d89cd

Please sign in to comment.