From dc3dda7d12662f3d7b5cb6c6c6366e07eae138fa Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Thu, 17 Oct 2024 13:24:06 +0200 Subject: [PATCH] [React@18] `useLayoutEffect` when setting value from a prop in `react-monaco-editor` (#195775) ## Summary This PR is part of https://github.com/elastic/kibana-team/issues/1016#issuecomment-2399310175 and needed to upgrade Kibana to React@18 in Legacy mode. We've found a problem in `react-monaco-editor` component which is a very thin wrapper around `monaco` where it doesn't play nicely with React@18 in legacy mode. You can read on details around the issue here https://github.com/elastic/eui/issues/8018 where we've found a similar issue in EUI's search bar component. The workaround we've found is to change `useEffect` -> `useLayouEffect` where the value that is coming from the prop is set into the model. The issue and a fix might be a bit controversal and the component is very thin, so I decided that it might be better to bring the fork into Kibana with only what's needed and with a workaround. --- NOTICE.txt | 26 ++ package.json | 1 - packages/kbn-test/jest-preset.js | 4 +- .../shared-ux/code_editor/impl/BUILD.bazel | 1 - .../shared-ux/code_editor/impl/README.mdx | 2 +- .../code_editor/impl/code_editor.test.tsx | 6 +- .../code_editor/impl/code_editor.tsx | 28 +- .../impl/react_monaco_editor/README.md | 33 +++ .../impl/react_monaco_editor/editor.tsx | 278 ++++++++++++++++++ .../impl/react_monaco_editor/index.ts | 10 + .../code_editor/mocks/monaco_mock/index.tsx | 4 +- yarn.lock | 7 - 12 files changed, 360 insertions(+), 40 deletions(-) create mode 100644 packages/shared-ux/code_editor/impl/react_monaco_editor/README.md create mode 100644 packages/shared-ux/code_editor/impl/react_monaco_editor/editor.tsx create mode 100644 packages/shared-ux/code_editor/impl/react_monaco_editor/index.ts diff --git a/NOTICE.txt b/NOTICE.txt index bdd6a95e57b04..9cd38e6773d88 100644 --- 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 index 2fc8ba6e22aef..87000cecd44a6 100644 --- 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 index 4f01bdcf27af1..791ee4a974823 100644 --- 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 index 24f18820496a4..848f5efad5303 100644 --- 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 index 8da5cf769c0c2..388483d73626d 100644 --- 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 index 8f775a5e572bc..628f89d4f159f 100644 --- 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 index deda021101791..5ab4d94bc9421 100644 --- 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 = ({ languageId, value, - onChange: _onChange, + onChange, width, height, options, @@ -225,8 +226,6 @@ export const CodeEditor: React.FC = ({ const [isHintActive, setIsHintActive] = useState(true); - const onChange = useBug175684OnChange(_onChange); - const startEditing = useCallback(() => { setIsHintActive(false); _editor?.focus(); @@ -708,23 +707,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(onChange); - useEffect(() => { - onChangePropRef.current = onChange; - }, [onChange]); - const onChangeWrapper = useCallback>((_value, event) => { - if (onChangePropRef.current) { - onChangePropRef.current(_value, event); - } - }, []); - - return onChangeWrapper; -}; - const UseBug177756ReBroadcastMouseDown: FC<{ children: React.ReactNode }> = ({ children }) => { const [$codeWrapper, setCodeWrapper] = React.useState(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 new file mode 100644 index 0000000000000..c9b8bbbb2c98c --- /dev/null +++ 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. diff --git a/packages/shared-ux/code_editor/impl/react_monaco_editor/editor.tsx b/packages/shared-ux/code_editor/impl/react_monaco_editor/editor.tsx new file mode 100644 index 0000000000000..c58e683a6b179 --- /dev/null +++ b/packages/shared-ux/code_editor/impl/react_monaco_editor/editor.tsx @@ -0,0 +1,278 @@ +/* eslint-disable @kbn/eslint/require-license-header */ +/** + * @notice + * 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. + */ + +import { monaco as monacoEditor, monaco } from '@kbn/monaco'; +import * as React from 'react'; +import { useEffect, useLayoutEffect, useMemo, useRef } from 'react'; + +export type EditorConstructionOptions = monacoEditor.editor.IStandaloneEditorConstructionOptions; + +export type EditorWillMount = (monaco: typeof monacoEditor) => void | EditorConstructionOptions; + +export type EditorDidMount = ( + editor: monacoEditor.editor.IStandaloneCodeEditor, + monaco: typeof monacoEditor +) => void; + +export type EditorWillUnmount = ( + editor: monacoEditor.editor.IStandaloneCodeEditor, + monaco: typeof monacoEditor +) => void | EditorConstructionOptions; + +export type ChangeHandler = ( + value: string, + event: monacoEditor.editor.IModelContentChangedEvent +) => void; + +export interface MonacoEditorProps { + /** + * Width of editor. Defaults to 100%. + */ + width?: string | number; + + /** + * Height of editor. Defaults to 100%. + */ + height?: string | number; + + /** + * The initial value of the auto created model in the editor. + */ + defaultValue?: string; + + /** + * Value of the auto created model in the editor. + * If you specify `null` or `undefined` for this property, the component behaves in uncontrolled mode. + * Otherwise, it behaves in controlled mode. + */ + value?: string | null; + + /** + * The initial language of the auto created model in the editor. Defaults to 'javascript'. + */ + language?: string; + + /** + * Theme to be used for rendering. + * The current out-of-the-box available themes are: 'vs' (default), 'vs-dark', 'hc-black'. + * You can create custom themes via `monaco.editor.defineTheme`. + */ + theme?: string | null; + + /** + * Optional string classname to append to the editor. + */ + className?: string | null; + + /** + * Refer to Monaco interface {monaco.editor.IStandaloneEditorConstructionOptions}. + */ + options?: monacoEditor.editor.IStandaloneEditorConstructionOptions; + + /** + * An event emitted before the editor mounted (similar to componentWillMount of React). + */ + editorWillMount?: EditorWillMount; + + /** + * An event emitted when the editor has been mounted (similar to componentDidMount of React). + */ + editorDidMount?: EditorDidMount; + + /** + * An event emitted before the editor unmount (similar to componentWillUnmount of React). + */ + editorWillUnmount?: EditorWillUnmount; + + /** + * An event emitted when the content of the current model has changed. + */ + onChange?: ChangeHandler; +} + +export function MonacoEditor({ + width = '100%', + height = '100%', + value, + defaultValue = '', + language = 'javascript', + theme, + options, + editorWillMount, + editorDidMount, + editorWillUnmount, + onChange, + className, +}: MonacoEditorProps) { + const containerElement = useRef(null); + + const editor = useRef(null); + + const _subscription = useRef(null); + + const __prevent_trigger_change_event = useRef(null); + + const fixedWidth = processSize(width); + + const fixedHeight = processSize(height); + + const onChangeRef = useRef(onChange); + onChangeRef.current = onChange; + + const style = useMemo( + () => ({ + width: fixedWidth, + height: fixedHeight, + }), + [fixedWidth, fixedHeight] + ); + + const handleEditorWillMount = () => { + const finalOptions = editorWillMount?.(monaco); + return finalOptions || {}; + }; + + const handleEditorDidMount = () => { + editorDidMount?.(editor.current!, monaco); + + _subscription.current = editor.current!.onDidChangeModelContent((event) => { + if (!__prevent_trigger_change_event.current) { + onChangeRef.current?.(editor.current!.getValue(), event); + } + }); + }; + + const handleEditorWillUnmount = () => { + editorWillUnmount?.(editor.current!, monaco); + }; + + const initMonaco = () => { + const finalValue = value !== null ? value : defaultValue; + + if (containerElement.current) { + // Before initializing monaco editor + const finalOptions = { ...options, ...handleEditorWillMount() }; + + const model = monaco.editor.createModel(finalValue!, language); + + editor.current = monaco.editor.create(containerElement.current, { + model, + ...(className ? { extraEditorClassName: className } : {}), + ...finalOptions, + ...(theme ? { theme } : {}), + }); + // After initializing monaco editor + handleEditorDidMount(); + } + }; + + // eslint-disable-next-line react-hooks/exhaustive-deps + useEffect(initMonaco, []); + + // useLayoutEffect instead of useEffect to mitigate https://github.com/facebook/react/issues/31023 in React@18 Legacy Mode + 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!, + }, + ], + // @ts-expect-error + undefined + ); + editor.current.pushUndoStop(); + __prevent_trigger_change_event.current = false; + } + }, [value]); + + useEffect(() => { + if (editor.current) { + const model = editor.current.getModel(); + monaco.editor.setModelLanguage(model!, language); + } + }, [language]); + + useEffect(() => { + if (editor.current) { + // Don't pass in the model on update because monaco crashes if we pass the model + // a second time. See https://github.com/microsoft/monaco-editor/issues/2027 + // @ts-expect-error + const { model: _model, ...optionsWithoutModel } = options; + editor.current.updateOptions({ + ...(className ? { extraEditorClassName: className } : {}), + ...optionsWithoutModel, + }); + } + }, [className, options]); + + useEffect(() => { + if (editor.current) { + editor.current.layout(); + } + }, [width, height]); + + useEffect(() => { + if (theme) { + monaco.editor.setTheme(theme); + } + }, [theme]); + + useEffect( + () => () => { + if (editor.current) { + handleEditorWillUnmount(); + editor.current.dispose(); + } + if (_subscription.current) { + _subscription.current.dispose(); + } + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [] + ); + + return
; +} + +MonacoEditor.displayName = 'MonacoEditor'; + +function processSize(size: number | string) { + return !/^\d+$/.test(size as string) ? size : `${size}px`; +} diff --git a/packages/shared-ux/code_editor/impl/react_monaco_editor/index.ts b/packages/shared-ux/code_editor/impl/react_monaco_editor/index.ts new file mode 100644 index 0000000000000..74bf5fca87a32 --- /dev/null +++ b/packages/shared-ux/code_editor/impl/react_monaco_editor/index.ts @@ -0,0 +1,10 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +export * from './editor'; diff --git a/packages/shared-ux/code_editor/mocks/monaco_mock/index.tsx b/packages/shared-ux/code_editor/mocks/monaco_mock/index.tsx index d62ab439a2a2e..d04eb38b8d1f4 100644 --- a/packages/shared-ux/code_editor/mocks/monaco_mock/index.tsx +++ b/packages/shared-ux/code_editor/mocks/monaco_mock/index.tsx @@ -8,8 +8,10 @@ */ import React, { useEffect, KeyboardEventHandler } from 'react'; -import { type MonacoEditorProps } from 'react-monaco-editor'; import { monaco } from '@kbn/monaco'; +// TODO: circular dependency +// import type { MonacoEditorProps } from '@kbn/code-editor/react_monaco_editor'; +type MonacoEditorProps = any; function createEditorInstance() { const keyDownListeners: Array<(e?: unknown) => void> = []; diff --git a/yarn.lock b/yarn.lock index fa3904c57939e..7861a19e57c2e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26945,13 +26945,6 @@ react-markdown@^6.0.3: unist-util-visit "^2.0.0" vfile "^4.0.0" -react-monaco-editor@^0.54.0: - version "0.54.0" - resolved "https://registry.yarnpkg.com/react-monaco-editor/-/react-monaco-editor-0.54.0.tgz#ec9293249a991b08264be723c1ec0ca3a6d480d8" - integrity sha512-9JwO69851mfpuhYLHlKbae7omQWJ/2ICE2lbL0VHyNyZR8rCOH7440u+zAtDgiOMpLwmYdY1sEZCdRefywX6GQ== - dependencies: - prop-types "^15.8.1" - react-popper-tooltip@^3.1.1: version "3.1.1" resolved "https://registry.yarnpkg.com/react-popper-tooltip/-/react-popper-tooltip-3.1.1.tgz#329569eb7b287008f04fcbddb6370452ad3f9eac"