apitech
/
Rocket.Chat
mirror of https://github.com/RocketChat/Rocket.Chat
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix: CodeMirror component not unmounting properly (#40902)

Browse Source
pull/40887/head^2
Tasso Evangelista 1 week ago committed by GitHub
parent a6bebd2581
commit a8a80873e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 227 additions and 74 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 5
      .changeset/bumpy-coats-reply.md
  2. 114
      apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.spec.tsx
  3. 178
      apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
  4. 4
      yarn.lock

5
.changeset/bumpy-coats-reply.md
Unescape View File

@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---
Fixes a memory leakage on the CodeMirror component (used by `code`-typed settings)

114
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.spec.tsx
Unescape View File

@ -0,0 +1,114 @@
import { act, render, waitFor } from '@testing-library/react';
import CodeMirror from './CodeMirror';
type ChangeHandler = (doc: { getValue: () => string }) => void;
const editor = {
on: jest.fn<void, [string, ChangeHandler]>(),
off: jest.fn<void, [string, ChangeHandler]>(),
setOption: jest.fn(),
setValue: jest.fn<void, [string]>(),
getValue: jest.fn<string, []>(),
toTextArea: jest.fn(),
};
const fromTextArea = jest.fn(() => editor);
jest.mock('codemirror', () => ({
__esModule: true,
default: { fromTextArea: (...args: unknown[]) => fromTextArea(...(args as [])) },
}));
jest.mock('codemirror/addon/edit/matchbrackets', () => ({}), { virtual: true });
jest.mock('codemirror/addon/edit/closebrackets', () => ({}), { virtual: true });
jest.mock('codemirror/addon/edit/matchtags', () => ({}), { virtual: true });
jest.mock('codemirror/addon/edit/trailingspace', () => ({}), { virtual: true });
jest.mock('codemirror/addon/search/match-highlighter', () => ({}), { virtual: true });
jest.mock('codemirror/lib/codemirror.css', () => ({}), { virtual: true });
jest.mock('../../../../../../../app/ui/client/lib/codeMirror/codeMirror', () => ({}), { virtual: true });
const flushAsync = () => act(() => Promise.resolve());
beforeEach(() => {
editor.on.mockClear();
editor.off.mockClear();
editor.setOption.mockClear();
editor.setValue.mockClear();
editor.getValue.mockReset();
editor.getValue.mockReturnValue('');
editor.toTextArea.mockClear();
fromTextArea.mockClear();
});
it('initializes CodeMirror on mount with the initial value', async () => {
render(<CodeMirror id='cm' readOnly={false} value='hello' onChange={jest.fn()} />);
await waitFor(() => expect(fromTextArea).toHaveBeenCalledTimes(1));
expect(editor.setValue).toHaveBeenCalledWith('hello');
expect(editor.on).toHaveBeenCalledWith('change', expect.any(Function));
});
it('tears down the editor on unmount', async () => {
const { unmount } = render(<CodeMirror id='cm' readOnly={false} value='' onChange={jest.fn()} />);
await waitFor(() => expect(fromTextArea).toHaveBeenCalledTimes(1));
unmount();
expect(editor.off).toHaveBeenCalledWith('change', expect.any(Function));
expect(editor.toTextArea).toHaveBeenCalledTimes(1);
});
it('updates options without recreating the editor', async () => {
const { rerender } = render(<CodeMirror id='cm' readOnly={false} mode='javascript' value='' onChange={jest.fn()} />);
await waitFor(() => expect(fromTextArea).toHaveBeenCalledTimes(1));
editor.setOption.mockClear();
rerender(<CodeMirror id='cm' readOnly mode='xml' value='' onChange={jest.fn()} />);
await flushAsync();
expect(fromTextArea).toHaveBeenCalledTimes(1);
expect(editor.toTextArea).not.toHaveBeenCalled();
expect(editor.setOption).toHaveBeenCalledWith('mode', 'xml');
expect(editor.setOption).toHaveBeenCalledWith('readOnly', true);
});
it('syncs external value changes into the editor', async () => {
const { rerender } = render(<CodeMirror id='cm' readOnly={false} value='a' onChange={jest.fn()} />);
await waitFor(() => expect(fromTextArea).toHaveBeenCalledTimes(1));
editor.getValue.mockReturnValue('a');
editor.setValue.mockClear();
rerender(<CodeMirror id='cm' readOnly={false} value='b' onChange={jest.fn()} />);
await flushAsync();
expect(editor.setValue).toHaveBeenCalledWith('b');
});
it('does not re-set the value when it already matches the editor content', async () => {
const { rerender } = render(<CodeMirror id='cm' readOnly={false} value='a' onChange={jest.fn()} />);
await waitFor(() => expect(fromTextArea).toHaveBeenCalledTimes(1));
editor.getValue.mockReturnValue('a');
editor.setValue.mockClear();
rerender(<CodeMirror id='cm' readOnly={false} value='a' onChange={jest.fn()} />);
await flushAsync();
expect(editor.setValue).not.toHaveBeenCalled();
});
it('forwards editor changes to onChange', async () => {
const onChange = jest.fn();
render(<CodeMirror id='cm' readOnly={false} value='' onChange={onChange} />);
await waitFor(() => expect(editor.on).toHaveBeenCalled());
const handler = editor.on.mock.calls[0][1];
act(() => handler({ getValue: () => 'typed' }));
expect(onChange).toHaveBeenCalledWith('typed');
});

178
apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx
Unescape View File

@ -1,9 +1,29 @@
import { useStableCallback } from '@rocket.chat/fuselage-hooks';
import type { Editor, EditorFromTextArea } from 'codemirror';
import { useCallback, useEffect, useRef, useState } from 'react';
import type { Editor, EditorConfiguration, EditorFromTextArea } from 'codemirror';
import { useEffect, useRef, useState } from 'react';
const defaultGutters = ['CodeMirror-linenumbers', 'CodeMirror-foldgutter'];
type CodeMirrorModule = typeof import('codemirror');
let codeMirrorPromise: Promise<CodeMirrorModule> | undefined;
const loadCodeMirror = (): Promise<CodeMirrorModule> => {
if (!codeMirrorPromise) {
codeMirrorPromise = Promise.all([
import('codemirror'),
import('../../../../../../../app/ui/client/lib/codeMirror/codeMirror'),
import('codemirror/addon/edit/matchbrackets'),
import('codemirror/addon/edit/closebrackets'),
import('codemirror/addon/edit/matchtags'),
import('codemirror/addon/edit/trailingspace'),
import('codemirror/addon/search/match-highlighter'),
import('codemirror/lib/codemirror.css'),
]).then(([cm]) => (cm as unknown as { default: CodeMirrorModule }).default ?? cm);
}
return codeMirrorPromise;
};
type CodeMirrorProps = {
id: string;
placeholder?: string;
@ -37,91 +57,105 @@ function CodeMirror({
showTrailingSpace = true,
highlightSelectionMatches = true,
readOnly,
value: valueProp,
value,
defaultValue,
onChange,
...props
}: CodeMirrorProps) {
const [value, setValue] = useState(valueProp || defaultValue);
const handleChange = useStableCallback(onChange);
const [textArea, setTextArea] = useState<HTMLTextAreaElement | null>(null);
const [codeMirror, setCodeMirror] = useState<CodeMirrorModule | null>(null);
const editorRef = useRef<EditorFromTextArea | null>(null);
const textAreaRef = useCallback(
async (node: HTMLTextAreaElement | null) => {
if (!node) return;
try {
const { default: CodeMirror } = await import('codemirror');
await Promise.all([
import('../../../../../../../app/ui/client/lib/codeMirror/codeMirror'),
import('codemirror/addon/edit/matchbrackets'),
import('codemirror/addon/edit/closebrackets'),
import('codemirror/addon/edit/matchtags'),
import('codemirror/addon/edit/trailingspace'),
import('codemirror/addon/search/match-highlighter'),
import('codemirror/lib/codemirror.css'),
]);
editorRef.current = CodeMirror.fromTextArea(node, {
lineNumbers,
lineWrapping,
mode,
gutters,
foldGutter,
matchBrackets,
autoCloseBrackets,
matchTags,
showTrailingSpace,
highlightSelectionMatches,
readOnly,
});
editorRef.current.on('change', (doc: Editor) => {
const newValue = doc.getValue();
setValue(newValue);
handleChange(newValue);
});
return () => {
if (node.parentNode) {
editorRef.current?.toTextArea();
}
};
} catch (error) {
// Latest-prop refs read by the init effect without forcing it to re-run.
const initialValueRef = useRef(value ?? defaultValue ?? '');
const optionsRef = useRef<EditorConfiguration>({});
optionsRef.current = {
lineNumbers,
lineWrapping,
mode,
gutters,
foldGutter,
matchBrackets,
autoCloseBrackets,
matchTags,
showTrailingSpace,
highlightSelectionMatches,
readOnly,
};
useEffect(() => {
let cancelled = false;
loadCodeMirror()
.then((mod) => {
if (!cancelled) setCodeMirror(() => mod);
})
.catch((error) => {
console.error('CodeMirror initialization failed:', error);
}
},
[
autoCloseBrackets,
foldGutter,
gutters,
highlightSelectionMatches,
lineNumbers,
lineWrapping,
matchBrackets,
matchTags,
mode,
handleChange,
readOnly,
showTrailingSpace,
],
);
});
return () => {
cancelled = true;
};
}, []);
useEffect(() => {
setValue(valueProp);
}, [valueProp]);
if (!textArea || !codeMirror) return;
const editor = codeMirror.fromTextArea(textArea, optionsRef.current);
editor.setValue(initialValueRef.current);
editorRef.current = editor;
const handleEditorChange = (doc: Editor) => {
handleChange(doc.getValue());
};
editor.on('change', handleEditorChange);
return () => {
editor.off('change', handleEditorChange);
editor.toTextArea();
editorRef.current = null;
};
}, [textArea, codeMirror, handleChange]);
useEffect(() => {
if (!editorRef.current) {
return;
}
const editor = editorRef.current;
if (!editor) return;
editor.setOption('lineNumbers', lineNumbers);
editor.setOption('lineWrapping', lineWrapping);
editor.setOption('mode', mode);
editor.setOption('gutters', gutters);
editor.setOption('foldGutter', foldGutter);
editor.setOption('matchBrackets', matchBrackets);
editor.setOption('autoCloseBrackets', autoCloseBrackets);
editor.setOption('matchTags', matchTags);
editor.setOption('showTrailingSpace', showTrailingSpace);
editor.setOption('highlightSelectionMatches', highlightSelectionMatches);
editor.setOption('readOnly', readOnly);
}, [
lineNumbers,
lineWrapping,
mode,
gutters,
foldGutter,
matchBrackets,
autoCloseBrackets,
matchTags,
showTrailingSpace,
highlightSelectionMatches,
readOnly,
]);
if (value !== editorRef.current.getValue()) {
editorRef.current.setValue(value ?? '');
useEffect(() => {
const editor = editorRef.current;
if (!editor) return;
const next = value ?? '';
if (editor.getValue() !== next) {
editor.setValue(next);
}
}, [textAreaRef, value]);
}, [value]);
return <textarea readOnly ref={textAreaRef} style={{ display: 'none' }} value={value} {...props} />;
return <textarea readOnly ref={setTextArea} style={{ display: 'none' }} {...props} />;
}
export default CodeMirror;

4
yarn.lock
Unescape View File

@ -11057,7 +11057,7 @@ __metadata:
"@react-aria/toolbar": "*"
"@rocket.chat/fuselage": "*"
"@rocket.chat/icons": "*"
"@rocket.chat/ui-client": 31.0.0-rc.0
"@rocket.chat/ui-client": 31.0.0
react: "*"
react-dom: "*"
languageName: unknown
@ -11328,7 +11328,7 @@ __metadata:
peerDependencies:
"@rocket.chat/layout": "*"
"@rocket.chat/tools": 0.3.0
"@rocket.chat/ui-contexts": 31.0.0-rc.0
"@rocket.chat/ui-contexts": 31.0.0
"@tanstack/react-query": "*"
react: "*"
react-hook-form: "*"

Write Preview
Loading…
Cancel
Save