-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[React@18]
useLayoutEffect
when setting value from a prop in `react…
…-monaco-editor` (#195775) ## Summary This PR is part of elastic/kibana-team#1016 (comment) 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 elastic/eui#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. (cherry picked from commit dc3dda7)
- Loading branch information
Showing
12 changed files
with
360 additions
and
40 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
packages/shared-ux/code_editor/impl/react_monaco_editor/README.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
Oops, something went wrong.