Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [React@18] `useLayoutEffect` when setting value from a prop in `react-monaco-editor` (#195775) #196671

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

…-monaco-editor` (elastic#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)
@kibanamachine kibanamachine merged commit 4849b41 into elastic:8.x Oct 17, 2024
36 checks passed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
@kbn/code-editor 0 1 +1

ESLint disabled line counts

id before after diff
@kbn/code-editor 3 5 +2

Total ESLint disabled count

id before after diff
@kbn/code-editor 3 6 +3

cc @Dosant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants