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

fixed issue of initializing editor multiple times #656

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

peonone
Copy link
Contributor

@peonone peonone commented Nov 16, 2022

It fixes #655, also addresses #640.

The fix is to initialize the editor (byinitMonaco) only when the component is mounted, and dispose the editor only before it's unmounted, as the props updating is already handled by perspective side effects.

@domoritz
Copy link
Member

Thank you. Can you fix the lint issues?

@domoritz
Copy link
Member

domoritz commented Nov 19, 2022

@KostiantynPopovych could you review this before I merge it since you sent #625?

@manooog, you also had some comments on the original pull request. Do you think this is a good change?

@peonone
Copy link
Contributor Author

peonone commented Nov 20, 2022

@domoritz Could you clarify about the lint issues? I re-ran yarn lint and nothing changed.
Not sure if you meant the warnings like

src/editor.tsx
   82:25  warning  React Hook useEffect has missing dependencies: 'className', 'defaultValue', 'handleEditorDidMount', 'handleEditorWillMount', 'language', 'options', 'overrideServices', 'theme', and 'value'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

These are expected, as we used those properties in the init side effect but don't want to trigger the effect again once any changed.

@domoritz
Copy link
Member

I'm not following. If we remove the array, does that change the behavior?

@peonone
Copy link
Contributor Author

peonone commented Nov 20, 2022

@domoritz Yes (ref),

  • removing the array parameter, it will be triggered for every re-rendering, works the same as componentDidUpdate
  • by empty array, it will be triggered only once after the component is mounted, same as componentDidMount

@domoritz
Copy link
Member

Then let's add a comment that suppressed the warning so we don't accidentally remove the array in the future.

@domoritz domoritz merged commit d8bd059 into react-monaco-editor:master Nov 22, 2022
@domoritz
Copy link
Member

Thank you. I published 0.51.0 with the fix.

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

Successfully merging this pull request may close these issues.

The editor gets initialized for multiple times
2 participants