[8.x] [React@18] `useLayoutEffect` when setting value from …

…a prop in `react-monaco-editor` (#195775) (#196671) # Backport This will backport the following commits from `main` to `8.x`: - [[React@18] `useLayoutEffect` when setting value from a prop in `react-monaco-editor` (#195775)](#195775)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
elastic · Oct 17, 2024 · 4849b41 · 4849b41
1 parent 52f92a8
commit 4849b41
Show file tree

Hide file tree

Showing 12 changed files with 360 additions and 40 deletions.
diff --git a/NOTICE.txt b/NOTICE.txt
@@ -214,6 +214,32 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
 
+---
+This code is forked from the `react-monaco-editor`
+https://github.com/react-monaco-editor/react-monaco-editor/blob/975cc47b5cb411ee2ffcbdb973daa9342e81a805/src/editor.tsx
+
+The MIT License (MIT)
+
+Copyright (c) 2016-present Leon Shi
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.
+
 ---
 This code is part of the Services provided by FullStory, Inc. For license information, please refer to https://www.fullstory.com/legal/terms-and-conditions/
 Portions of this code are licensed under the following license:

diff --git a/package.json b/package.json
@@ -1218,7 +1218,6 @@
     "react-intl": "6.6.6",
     "react-is": "^17.0.2",
     "react-markdown": "^6.0.3",
-    "react-monaco-editor": "^0.54.0",
     "react-popper-tooltip": "^3.1.1",
     "react-recompose": "^0.33.0",
     "react-redux": "^7.2.8",

diff --git a/packages/kbn-test/jest-preset.js b/packages/kbn-test/jest-preset.js
@@ -105,9 +105,9 @@ module.exports = {
 
   // An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation
   transformIgnorePatterns: [
-    // ignore all node_modules except monaco-editor, monaco-yaml and react-monaco-editor which requires babel transforms to handle dynamic import()
+    // ignore all node_modules except monaco-editor, monaco-yaml which requires babel transforms to handle dynamic import()
     // since ESM modules are not natively supported in Jest yet (https://github.com/facebook/jest/issues/4842)
-    '[/\\\\]node_modules(?![\\/\\\\](byte-size|monaco-editor|monaco-yaml|monaco-languageserver-types|monaco-marker-data-provider|monaco-worker-manager|vscode-languageserver-types|react-monaco-editor|d3-interpolate|d3-color|langchain|langsmith|@cfworker|gpt-tokenizer|flat|@langchain))[/\\\\].+\\.js$',
+    '[/\\\\]node_modules(?![\\/\\\\](byte-size|monaco-editor|monaco-yaml|monaco-languageserver-types|monaco-marker-data-provider|monaco-worker-manager|vscode-languageserver-types|d3-interpolate|d3-color|langchain|langsmith|@cfworker|gpt-tokenizer|flat|@langchain))[/\\\\].+\\.js$',
     'packages/kbn-pm/dist/index.js',
     '[/\\\\]node_modules(?![\\/\\\\](langchain|langsmith|@langchain))/dist/[/\\\\].+\\.js$',
     '[/\\\\]node_modules(?![\\/\\\\](langchain|langsmith|@langchain))/dist/util/[/\\\\].+\\.js$',

diff --git a/packages/shared-ux/code_editor/impl/BUILD.bazel b/packages/shared-ux/code_editor/impl/BUILD.bazel
@@ -24,7 +24,6 @@ SRCS = glob(
 BUNDLER_DEPS = [
   "@npm//react",
   "@npm//tslib",
-  "@npm//react-monaco-editor",
 ]
 
 js_library(

diff --git a/packages/shared-ux/code_editor/impl/README.mdx b/packages/shared-ux/code_editor/impl/README.mdx
@@ -9,7 +9,7 @@ date: 2022-12-05
 
 ## Description
 
-This component is an abstraction of the [Monaco Code Editor](https://microsoft.github.io/monaco-editor/) (and the [React Monaco Editor component](https://github.com/react-monaco-editor/react-monaco-editor)). This component still allows access to the other Monaco features.
+This component is an abstraction of the [Monaco Code Editor](https://microsoft.github.io/monaco-editor/). This component still allows access to the other Monaco features.
 
 ## Usage
 

diff --git a/packages/shared-ux/code_editor/impl/code_editor.test.tsx b/packages/shared-ux/code_editor/impl/code_editor.test.tsx
@@ -18,10 +18,8 @@ import { MockedMonacoEditor, mockedEditorInstance } from '@kbn/code-editor-mock/
 
 import { CodeEditor } from './code_editor';
 
-jest.mock('react-monaco-editor', () => {
-  return function JestMockEditor() {
-    return MockedMonacoEditor;
-  };
+jest.mock('./react_monaco_editor', () => {
+  return { MonacoEditor: MockedMonacoEditor };
 });
 
 // Mock the htmlIdGenerator to generate predictable ids for snapshot tests

diff --git a/packages/shared-ux/code_editor/impl/code_editor.tsx b/packages/shared-ux/code_editor/impl/code_editor.tsx
@@ -8,9 +8,6 @@
  */
 
 import React, { useState, useRef, useCallback, useMemo, useEffect, KeyboardEvent, FC } from 'react';
-import ReactMonacoEditor, {
-  type MonacoEditorProps as ReactMonacoEditorProps,
-} from 'react-monaco-editor';
 import {
   htmlIdGenerator,
   EuiToolTip,
@@ -34,6 +31,10 @@ import {
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
 import { css } from '@emotion/react';
+import {
+  MonacoEditor as ReactMonacoEditor,
+  type MonacoEditorProps as ReactMonacoEditorProps,
+} from './react_monaco_editor';
 import './register_languages';
 import { remeasureFonts } from './remeasure_fonts';
 
@@ -168,7 +169,7 @@ export interface CodeEditorProps {
 export const CodeEditor: React.FC<CodeEditorProps> = ({
   languageId,
   value,
-  onChange: _onChange,
+  onChange,
   width,
   height,
   options,
@@ -225,8 +226,6 @@ export const CodeEditor: React.FC<CodeEditorProps> = ({
 
   const [isHintActive, setIsHintActive] = useState(true);
 
-  const onChange = useBug175684OnChange(_onChange);
-
   const startEditing = useCallback(() => {
     setIsHintActive(false);
     _editor?.focus();
@@ -701,23 +700,6 @@ const useFitToContent = ({
   }, [editor, isFitToContent, minLines, maxLines, isFullScreen]);
 };
 
-// https://github.com/elastic/kibana/issues/175684
-// 'react-monaco-editor' has a bug that it always calls the initial onChange callback, so the closure might become stale
-// we work this around by calling the latest onChange from props
-const useBug175684OnChange = (onChange: CodeEditorProps['onChange']) => {
-  const onChangePropRef = useRef<CodeEditorProps['onChange']>(onChange);
-  useEffect(() => {
-    onChangePropRef.current = onChange;
-  }, [onChange]);
-  const onChangeWrapper = useCallback<NonNullable<CodeEditorProps['onChange']>>((_value, event) => {
-    if (onChangePropRef.current) {
-      onChangePropRef.current(_value, event);
-    }
-  }, []);
-
-  return onChangeWrapper;
-};
-
 const UseBug177756ReBroadcastMouseDown: FC<{ children: React.ReactNode }> = ({ children }) => {
   const [$codeWrapper, setCodeWrapper] = React.useState<HTMLElement | null>(null);
 

diff --git a/packages/shared-ux/code_editor/impl/react_monaco_editor/README.md b/packages/shared-ux/code_editor/impl/react_monaco_editor/README.md
@@ -0,0 +1,33 @@
+This is a fork of [react-monaco-editor project](https://github.com/react-monaco-editor/react-monaco-editor) that is a Monaco editor wrapper for React.
+This fork is needed to apply a change that fixes the editor behavior in Kibana when running React@18 in Legacy Mode and the bug is described [here](https://github.com/facebook/react/issues/31023)
+The change is to replace the `useEffect` hook with `useLayoutEffect` when the editor is in controlled mode and the value is updated from prop.
+
+```diff
+---useEffect(() => {
++++useLayoutEffect(() => {
+    if (editor.current) {
+      if (value === editor.current.getValue()) {
+        return;
+      }
+
+      const model = editor.current.getModel();
+      __prevent_trigger_change_event.current = true;
+      editor.current.pushUndoStop();
+      // pushEditOperations says it expects a cursorComputer, but doesn't seem to need one.
+      model.pushEditOperations(
+        [],
+        [
+          {
+            range: model.getFullModelRange(),
+            text: value,
+          },
+        ],
+        undefined,
+      );
+      editor.current.pushUndoStop();
+      __prevent_trigger_change_event.current = false;
+    }
+  }, [value]);
+```
+
+In addition, the fork only includes functionality that is used in Kibana and removes the rest of the code that is not needed.