ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Schema addChildCheck/addAttributeCheck filtering

Open map-r opened this issue 1 year ago • 2 comments

Suggested merge commit message (convention)

Feature (engine): `Schema#addChildCheck()` and `Schema#addAttributeCheck()` can now register a callback for specific item or attribute, which should improve performance when using custom callback checks. Callback checks should be added only for specific item or attribute if possible. See the API reference. Closes #15834.

Fix (engine): `Schema#checkChild()` will now correctly check custom callback checks for each item in the context.

Internal: Used specific callback checks in some of the features.

Docs (engine): Improved schema deep dive guide and added missing section related to attributes custom callback checks.


Additional information

map-r avatar Mar 01 '24 09:03 map-r

@DawidKossowski Outro for you and of course for @scofalik:

I've fixed and resolved some of the comments. The major one was to move the callback triggering into an actual checkChild and checkAttribute function - I admit , I took it naturally that it should probably be kept where it was before, i.e. in the prioritized listeners. But Szymon actually was right that he meant something else, simpler, and it does the work so far, too (didn't yet come across any biggest issue with that, but that yet may wait to be discovered).

The things left out for you to pick up are:

  • decision what to do with the type of filtering, whether it should be done via context and children both, or only by children (rest would be generic), I proposed a solution to use an object-based parameter that we will extract correct parameters to filter for any one of these properties, giving us better control over when to trigger the check.
  • an useful thing to add in the documentation about precedences of custom callbacks also over the disallow* and allow* rules.

map-r avatar Mar 13 '24 12:03 map-r

Summary:

  1. Schema#addChildCheck() and Schema#addAttributeCheck() have a new optional parameter that allows to register the callback for a specific item or specific attribute.
  2. Earlier, when you registered a custom callback, it was fired for every checkChild() and checkAttribute() call. With multiple features and multiple custom callbacks, it could lead to performance problems (lagging when typing).
  3. The way how it works is very simple:
    1. If you registered addChildCheck( cb, 'foo' ) it will be called when checkChild( ctx, 'foo' ) is called.
      1. Mind, that checkChild( ctx ) also checks the whole ctx. If 'foo' is inside ctx the callback will be fired too.
    2. If you registered addAttributeCheck( cb, 'bold' ) it will be called when checkAttribute( ctx, 'bold' ) is called.
      1. Mind, that you register attribute check for a model attribute not a model item.
  4. Generic callbacks are still available.
    1. Of course, generic callbacks are not recommended as they can lead to performance problems. However, sometimes, they can't be avoided.
  5. checkChild() and checkAttribute() are still decorated so you can add listeners to the events. This is not recommended as it can also lead to performance problems.
  6. The order of performing checks is as follows:
    1. All callbacks added as events listeners with highest and high priority.
    2. All generic callbacks. Among them, order as added in code.
    3. All specific callbacks. Among them, order as added in code.
    4. Declarative checks.

I also added this summary to the issue so it can be easy to found.

scofalik avatar Jun 10 '24 11:06 scofalik