Warn when normalization removes node

[jameshfisher]

Description
Slate requires the invariant that children are all blocks or all inlines. It enforces this in default normalization by removing children. When such a node is removed, it's almost certainly due to a programming error: the developer needs to fix their application to ensure it maintains this invariant. But currently Slate does not tell the developer this.

I made such a programming error, and spent a long time debugging nodes that mysteriously went missing. I would have fixed it within 30 seconds if Slate had warned me when it detected this error.

Context
Note I have used console.warn despite the eslint rule. As far as I can see, Slate has no other facility for runtime warnings.

Checks

• The new code matches the existing patterns and styles.
• The tests pass with yarn test.
• The linter passes with yarn lint. (Fix errors with yarn fix.)
• The relevant examples still work. (Run examples with yarn start.)
• You've added a changeset if changing functionality. (Add one with yarn changeset add.)

[changeset-bot]

🦋 Changeset detected

Latest commit: 1ee0fb1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
slate	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dylans]

Excellent suggestion, I've been a victim of this as well.

[TheSpyder]

I disagree with how this is implemented. Strongly. Slate core does not emit warnings; that lint rule is there for a reason.

The core is used by a variety of editors, not just slate-react, and even though this might be caused by programmer error and good to highlight we can't know that it is appropriate to log a console message.

In particular, collaborative editors can end up in all sorts of weird states that require normalization to fix. This is normal behaviour, I would like to see this reverted (or I can do it) and we look for another way to handle the scenario. Some vague ideas:

• sample code that, during normalization, warns if editor.apply is called with a remove_node operation (admittedly this would not be able to offer information about why it was removed)
• start building a system for runtime warning messages, perhaps using weak references so they don't cause memory leaks
• some sort of "development" mode to help ensure warnings are not shipped to production

[dylans]

@TheSpyder ok, no problem, reverting now. I would vote for option 2 if feasible.

[jameshfisher]

Slate core does not emit warnings; that lint rule is there for a reason.

@TheSpyder what's the reason, more explicitly? That you don't want warnings emitted in production?

The point about collaborative editing is interesting - I'm not dealing with those kinds of problems.

For context, here's how React does it. I think this resembles @TheSpyder's option 3. It seems to work well. Perhaps we can use/copy/steal it.

[TheSpyder]

@TheSpyder what's the reason, more explicitly? That you don't want warnings emitted in production?

Pretty much. Slate is a framework to build editors, and I don’t like frameworks that emit console logs.

For context, here's how React does it. I think this resembles @TheSpyder's option 3. It seems to work well. Perhaps we can use/copy/steal it.

React also has separate production and development builds. I’m not sure we want to go that far.

I guess the process.env.NODE_ENV approach is fairly common? We might need to be careful about introducing a check for that though.