Ckeditor5: Introduce Node/DF#is method

[Reinmar]

We had such method in CKE4, but it was only available for elements: el.is( 'p' ). It's just a bit nicer than el.name == 'p', but a lot nicer than el.name == 'p' || el.name == 'div'.

What I've been thinking of is to make this method even more useful, by allowing checking whether the node is a text node or document fragment or whatever else.

There are many algorithms which need to verify what kind of node they are dealing with. We've seen e.g. the whole discussion around https://github.com/ckeditor/ckeditor5-engine/issues/807.

If we had a nicer API to check the type of the node or DF, then we'd be more pleased to use it.

Some ideas:

// If called with one arg, checks if this is an element with a given name:
node.is( 'p' );
// Equals to:
node.is( 'element', 'p' );

// Other types:
node.is( 'documentFragment' );
node.is( 'text' );
node.is( 'textProxy' );
node.is( 'text', 'expected data?' ); // not sure about this

// Matching more elements names:
node.is( [ 'p', 'div' ] ); // I don't like passing an array, so maybe simply...
node.is( 'p,div' );

Such method will have a lot of pros:

1. Less code.
2. More readable code.
3. Less imports.
4. Less potential circular deps.

[Reinmar]

Example how code would be simplified: https://github.com/ckeditor/ckeditor5-image/pull/48/files#r100029767

[scofalik]

Either you implement is using ducktyping or you have to import everything in Node to check instanceof -- this won't solve circ deps, it will make it even worse.

The idea is neat and might clean code, but I am afraid it will require some dirty ducktyping or properties like type...

[Reinmar]

Either you implement is using ducktyping or you have to import everything in Node to check instanceof -- this won't solve circ deps, it will make it even worse.

Good point.

or properties like type...

Yep, exactly! I've been proposing this property long time ago anyway :)

Checking the types by the property and not instance of will also prevent some nasty issues cause by duplicated engine modules (may happen in some broken cases).

[scofalik]

I mean.. Okay, I can live with this. This is language fault, not our fault, that importing is difficult and to check types we have to do such silly things.

Still, I'd be more happy if type property did not exist :)

[pjasiun]

Or the is method might not be implemented only in node but in each class separately. Note that is( 'p' ) on element check tag name, but on text will return false.

[scofalik]

This is actually good idea.

[pjasiun]

BTW: it reminds me that we have a view.Matcher class. Having the is method we can make it work not only on the element but also on element types (can match all elements of given type).

[pjasiun]

But to be honest I have mixed feelings about the Matcher class. Usually, it's more convenient for me to use a simple callback.

[scofalik]

I'll implement this feature using @pjasiun idea.

Of course (?) is method will be "inherited" like classes (so element.is( 'node' ) will return true).

[scofalik]

Do we bring those for view too?

[Reinmar]

Do we bring those for view too?

I guess we'd like to keep this consistent.

Of course (?) is method will be "inherited" like classes (so element.is( 'node' ) will return true).

I'm not sure about that. This doesn't seem to be necessary. If there's is() method, then you can assume it's a node already. Let's KISS.

[scofalik]

You are right about Node. What I mean is -- every class that extends Element should return true for .is( 'element' ).