slate: nested validation stacks break because they've already been normalized

Do you want to request a feature or report a bug?

Bug.

What’s the current behavior?

If you write a custom normalize function for a schema, and it happens to do something like change.moveNodeByKey, there’s a chance that calling that change method will initialize its own validation stack, fixing problems that the parent validation stack assumes are still there.

For example, consider that you have two child text nodes adjacent to each other, which is invalid in Slate. If in your normalize function you make any change to the parent of those text nodes, that change will end up normalizing internally, and the text nodes will be combined. However, the parent validation stack still thinks that an invalid rule that needs to be fixed, so it tries to normalize them again, and fails.

What’s the expected behavior?

Not sure.

The ideal would be to prevent the double-checking from happening. I haven’t dug into this yet, so I’m not sure if it’s impossible to do or not.

Another option would be to automatically pass options.normalize: false to all change methods called while performing a validations. The “core” validateNode handlers already do this automatically. But this feels like it might be too magic-y?

Have others run into this? Thoughts?

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 1
Comments: 15 (13 by maintainers)

Commits related to this issue

refactor normalization to be operations-based (#2193) #### Is this adding or improving a _feature_ or fixing a _bug_? Improvement. #### What's the new behavior? This changes the normalizatio... — committed to ianstormtaylor/slate by ianstormtaylor 6 years ago
refactor normalization to be operations-based (#2193) #### Is this adding or improving a _feature_ or fixing a _bug_? Improvement. #### What's the new behavior? This changes the normalizatio... — committed to jtadmor/slate by ianstormtaylor 6 years ago

Most upvoted comments

This can be solved by using deferNormalization, in combination with improvements to the performance from https://github.com/ianstormtaylor/slate/issues/2136.

ianstormtaylor on Aug 28, 2018

Pretty sure I’m running into this with wrapBlockByKey. Here’s an example trace (pardon webpack indirection) that shows the multiple normalization passes with wrapBlockByKey calling moveNodeByKey internally. In the trace, it’s isolated down to a single schema rule and a single validation failure:

Uncaught Error: Could not find a node with key "3".
    at Document.assertNode (http://localhost:3000/static/js/bundle.js:204111:15)
    at Document.getPath (http://localhost:3000/static/js/bundle.js:205363:24)
    at Document.object.(anonymous function) [as getPath] (http://localhost:3000/static/js/bundle.js:210941:30)
    at ./node_modules/slate/lib/changes/by-key.js.Changes.mergeNodeByKey (http://localhost:3000/static/js/bundle.js:199649:23)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as mergeNodeByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at http://localhost:3000/static/js/bundle.js:201370:16
    at List.__iterate (http://localhost:3000/static/js/bundle.js:30578:13)
    at List.forEach (http://localhost:3000/static/js/bundle.js:32753:19)
    at http://localhost:3000/static/js/bundle.js:201369:26
    at iterate (http://localhost:3000/static/js/bundle.js:201084:5)
    at normalizeNode (http://localhost:3000/static/js/bundle.js:201105:3)
    at normalizeNodeAndChildren (http://localhost:3000/static/js/bundle.js:201047:5)
    at _loop (http://localhost:3000/static/js/bundle.js:201020:7)
    at normalizeNodeAndChildren (http://localhost:3000/static/js/bundle.js:201042:5)
    at ./node_modules/slate/lib/changes/with-schema.js.Changes.normalizeNodeByKey (http://localhost:3000/static/js/bundle.js:200975:3)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as normalizeNodeByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at ./node_modules/slate/lib/changes/by-key.js.Changes.moveNodeByKey (http://localhost:3000/static/js/bundle.js:199700:12)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as moveNodeByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at ./node_modules/slate/lib/changes/by-key.js.Changes.wrapBlockByKey (http://localhost:3000/static/js/bundle.js:200181:10)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as wrapBlockByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at Object.normalize (http://localhost:3000/static/js/bundle.js:226701:100)
    at http://localhost:3000/static/js/bundle.js:207420:34
    at iterate (http://localhost:3000/static/js/bundle.js:201084:5)
    at normalizeNode (http://localhost:3000/static/js/bundle.js:201105:3)
    at normalizeNodeAndChildren (http://localhost:3000/static/js/bundle.js:201047:5)
    at _loop (http://localhost:3000/static/js/bundle.js:201020:7)
    at normalizeNodeAndChildren (http://localhost:3000/static/js/bundle.js:201042:5)
    at ./node_modules/slate/lib/changes/with-schema.js.Changes.normalizeNodeByKey (http://localhost:3000/static/js/bundle.js:200975:3)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as normalizeNodeByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at ./node_modules/slate/lib/changes/by-key.js.Changes.moveNodeByKey (http://localhost:3000/static/js/bundle.js:199700:12)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as moveNodeByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at ./node_modules/slate/lib/changes/by-key.js.Changes.wrapBlockByKey (http://localhost:3000/static/js/bundle.js:200181:10)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as wrapBlockByKey] (http://localhost:3000/static/js/bundle.js:202161:15)
    at Object.normalize (http://localhost:3000/static/js/bundle.js:226701:100)
    at http://localhost:3000/static/js/bundle.js:207420:34
    at iterate (http://localhost:3000/static/js/bundle.js:201084:5)
    at normalizeNode (http://localhost:3000/static/js/bundle.js:201105:3)
    at normalizeNodeAndChildren (http://localhost:3000/static/js/bundle.js:201047:5)
    at _loop (http://localhost:3000/static/js/bundle.js:201020:7)
    at normalizeNodeAndChildren (http://localhost:3000/static/js/bundle.js:201042:5)
    at ./node_modules/slate/lib/changes/with-schema.js.Changes.normalizeNodeByKey (http://localhost:3000/static/js/bundle.js:200975:3)
    at Change.call (http://localhost:3000/static/js/bundle.js:202102:10)
    at Change.(anonymous function) [as normalizeNodeByKey] (http://localhost:3000/static/js/bundle.js:202161:15)

This is a blocker for some of the more complex operations (most of them) in my schema rules. I can’t seem to find a way around it. Maybe validateNode is best for now on the stuff that may trigger this, until there’s a way to prevent the extra normalization?

I’m not sure how I feel about needing to pass options.normalize: false on every change method (could get redundant). Feels kind of clunky compared to the rest of the lib, especially when chaining lots of methods. Maybe the change object itself could be callable and take a normalize option, then return a new change object internally configured not to normalize when its change methods are called?

So instead of:

change
  .removeTextByKey(textNode.key, 0, length, { normalize: false })
  .setNodeByKey(node.key, 'title', { normalize: false })
  .wrapBlockByKey(node.key, 'folder', { normalize: false });

It could be:

change({ normalize: false })
  .removeTextByKey(textNode.key, 0, length)
  .setNodeByKey(node.key, 'title')
  .wrapBlockByKey(node.key, 'folder');

But, then, in that case is it not mutating the change object that ultimately gets applied? If it has to be the original change throughout the rule, maybe something semantically along the lines of:

change.pauseNormalization();
change
  .removeTextByKey(textNode.key, 0, length)
  .setNodeByKey(node.key, 'title')
  .wrapBlockByKey(node.key, 'folder');
change.resumeNormalization();

That almost looks like a context manager. Is there syntax for that in js?

It definitely wouldn’t feel too magical to me to have normalization always disabled inside schema rules, if there were a way to still have the nodes undergo internal normalization after the fact. Is that possible? Maybe there would be a way to collect keys where normalizeNodeByKey was called within a schema rule, then attempt to normalize those keys after the rule normalization is finished? Not sure if that could get too weird with the rule normalization already having performed its own changes.

alanchrt on Nov 11, 2017