[pull] develop from RocketChat:develop

.changeset/bumpy-coats-reply.md

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

apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.spec.tsx

@@ -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');
+});

apps/meteor/client/views/admin/settings/Setting/inputs/CodeMirror/CodeMirror.tsx

@@ -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;

yarn.lock

@@ -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: "*"