suren-atoyan / monaco-react

Monaco Editor for React - use the monaco-editor in any React application without needing to use webpack (or rollup/parcel/etc) configuration files / plugins
https://monaco-react.surenatoyan.com/
MIT License
3.75k stars 266 forks source link

ControlledEditor #8

Closed JeremyGrieshop closed 5 years ago

JeremyGrieshop commented 5 years ago

I've found it pretty useful at times to have a controlled Editor. the react-codemirror2 project has one, too. Here's a sandbox demonstrating a ControlledEditor wrapping your Editor component:

https://codesandbox.io/embed/controllededitor-monaco-editorreact-dfkf5

suren-atoyan commented 5 years ago

You are totally right, it can be really useful to have it controlled. And of course, we thought about it. The question is, on which we haven't answered yet: will there be performance issues in very large codebases or not, and the second: should we do it in this library according to keeping the nature of Monaco as much as it is possible (just keeping it uncontrolled - in this case we need to rename the "value" prop name), and considering that fact that the user/developer can do it easily as you demonstrated. So, we haven’t decided yet; glad to hear feedback about it.

JeremyGrieshop commented 5 years ago

Yes, absolutely, there are caveats that the end users would have to understand. The way that codemirror2 did it was by exporting 2 named components: Controlled and Uncontrolled, so the user of the component makes the active choice of which one to use. You could even "default export" the controlled one, so it works exactly the same why it always has.

suren-atoyan commented 5 years ago

Ah, that's a great idea to export separate components for uncontrolled and controlled components. The main worry of mine about it was: if we will make the Editor component as "usual" React component, which is going to be controlled when "value" prop is provided, so, the most of users/developers will do it automatically, even if they don't need it. By exporting two separate components we will be sure that when someone uses the controlled one, it means he really needs it.

Okay, I’ll design it. Will keep up to date here.

JeremyGrieshop commented 5 years ago

So, your current Editor is uncontrolled, which I would assume would be the default, as it wouldn't break anything that currently uses it. Yes, by having 2, the user has to know what he's doing by explicitly importing the Controlled one. With react codemirror, I believe they even have the concept with the uncontrolled component that the value that is passed in is only the "initial" value, and the uncontrolled component stops paying attention to updates from it after initial mount.

suren-atoyan commented 5 years ago

I examined a little bit react-codemirror2. I like the idea for a separate controlled component. I’ll work on it this week; we will have Editor (uncontrolled) component as a default export, DiffEditor (uncontrolled) as named export, and ControlledEditor / ControlledDiffEditor as two more named exports. I'll add more details in the documentation.

suren-atoyan commented 5 years ago

It has already been implemented in the new version (1.0.8). You can check the new README file to see how you can use it. Thank you!