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

A way to disable contenteditable warnings #5837

Closed
devongovett opened this issue Jan 12, 2016 · 16 comments
Closed

A way to disable contenteditable warnings #5837

devongovett opened this issue Jan 12, 2016 · 16 comments
Milestone

Comments

@devongovett
Copy link
Contributor

warning(
!props.contentEditable || props.children == null,
'A component is `contentEditable` and contains `children` managed by ' +
'React. It is now your responsibility to guarantee that none of ' +
'those nodes are unexpectedly modified or duplicated. This is ' +
'probably not intentional.'
);

React warns:

A component is contentEditable and contains children managed by
React. It is now your responsibility to guarantee that none of
those nodes are unexpectedly modified or duplicated. This is
probably not intentional.

I am writing a rich text editor using React, and have written it to ensure that the nodes are not modified by the browser by capturing all input events, performing the modifications on my internal document format, and re-rendering using React. However, on each render, React still warns me about this.

I would be awesome to be able to disable this warning somehow, such as with a specific value to the contentEditable prop, another name or something. I'm not sure the best approach to take here, so I thought I'd create an issue for discussion around this.

@mjgil
Copy link

mjgil commented Feb 2, 2016

+1

@jimfb
Copy link
Contributor

jimfb commented Feb 2, 2016

Duplicate of #4662. The warning is useful in most cases. I'm pretty sure you get a paste event for nodes that are not contentEditable, so if you are capturing all the events and preventing the browser changes, why do you have contentEditable specified?

To avoid the warning, you can bail out of React (#2477 (comment)) and then call React.render() in your contentEditable container node, which will not cause us to warn (afaik).

@devongovett
Copy link
Contributor Author

It needs to be contentEditable in order to enable the browser cursor behavior, input events, etc. If you render at a different mount point, preventing events from bubbling up to parent components no longer works correctly (see #1691).

As a stopgap solution I ended up overriding console.error in my app and ignoring messages that match a regex. Pretty silly, but it works for now, and of course it's not enabled in production anyway. I'd definitely love to have an option on the component to say "hey react, I know what I'm doing" though.

@jimfb
Copy link
Contributor

jimfb commented Feb 2, 2016

You can capture input events without content editable (http://jsfiddle.net/vm9gjhs3/) and can simulate the cursor behavior yourself (which you basically need to do anyway in order to implement a good editor, as per https://medium.com/medium-eng/why-contenteditable-is-terrible-122d8a40e480#6497). The browsers all have different behaviors with regards to how cursor position works. If you want a truly nice editor, you will end up using almost nothing from contentEditable. But I get it, you're trying to reuse the browser's functionality under the (very optimistic) assumption that contentEditable gets you most of the way there.

With regards to your hack: A better stopgap solution (hack) would probably be to set contentEditable imperatively (as demonstrated in my fiddle: http://jsfiddle.net/4q2p6ug8/). That way, React doesn't even know you've set it, and you can share your component without overwriting other people's console.error method.

@devongovett
Copy link
Contributor Author

Yeah, that's the google docs method: reimplement everything. However for non-word processors, it is possible to get most of the way there using content editable and some code to manage the selection. Even medium and the new Facebook notes (made with react) still use content editable to some degree.

I like that new hack. Much better. Thanks.

@mxstbr
Copy link
Contributor

mxstbr commented Feb 24, 2016

@jimfb @spicyj this seems like something I could take a look at!

Have there been any (internal) discussion what such an opt-out would look like? I could imagine it being it's own function exposed through React (similar to React.initializeTouchEvents(true) back in the day).

React.disableContentEditableWarning(true);

or another property specified on the component itself (could make it a bit hard to work with on purpose, similar to dangerouslySetInnerHTML)

<MyComponent contentEditable disableContentEditableWarning />

The benefit, and also drawback, of the first option is that it'd be app-wide, whereas the second one would have to be specified on every contentEditable component.

@jimfb
Copy link
Contributor

jimfb commented Feb 24, 2016

We generally avoid global configuration like the plague, so we would almost certainly prefer that it be specified on the component itself. At first glance, I kind of like your second proposal. I'll bikeshed a little with Ben and see what we come up with.

@sophiebits
Copy link
Collaborator

My two best naming proposals right now are silenceContentEditableWarning and allowContentEditableChildren, both of which I like a little more than disableContentEditableWarning.

@devongovett
Copy link
Contributor Author

How about dangerouslyAllowContentEditableChildren? Mirrors dangerouslySetInnerHTML.

@jimfb
Copy link
Contributor

jimfb commented Feb 24, 2016

I like "suppress" better than "silence". Or "allow" is also fine, but the problem with allowContentEditableChildren is that it implies a possible change in behavior, rather than merely disabling the warning.

For this reason, I think I'm in favor of suppressContentEditableWarning or contentEditableWarning="disabled"

mxstbr added a commit to mxstbr/react that referenced this issue Feb 24, 2016
mxstbr added a commit to mxstbr/react that referenced this issue Feb 24, 2016
mxstbr added a commit to mxstbr/react that referenced this issue Feb 24, 2016
@jimfb
Copy link
Contributor

jimfb commented Feb 24, 2016

Fixed in #6112

@jimfb jimfb closed this as completed Feb 24, 2016
@mxstbr
Copy link
Contributor

mxstbr commented Feb 24, 2016

Should we document this in the content editable doc?

@sophiebits
Copy link
Collaborator

If you want to send a PR, sure.

@ra30r
Copy link

ra30r commented Apr 2, 2022

const nativeConsoleError = console.error;
console.error = function(m, at){
//console.warn({args: arguments})

if(m === "Warning: A component is contentEditable and contains children managed by React. It is now your responsibility to guarantee that none of those nodes are unexpectedly modified or duplicated. This is probably not intentional.%s"){
// this shit is redundant and should have an opt out
// since I may happen to accept 'responsibility to guarantee'
}
else {
nativeConsoleError(m, at);
}
}

@gaearon
Copy link
Collaborator

gaearon commented Apr 2, 2022

It does have an opt-out, as linked from this thread. The opt-out prop is called suppressContentEditableWarning.

@y1n0
Copy link

y1n0 commented Mar 27, 2024

is there a way to disable this warning globally instead of having to add the suppressContentEditableWarning prop to every component?
Apparently React.disableContentEditableWarning(true) which @mxstbr mentioned is not a thing yet but it looks very pretty

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

No branches or pull requests

8 participants