Add is() method to other model/view objects

[Reinmar]

After https://github.com/ckeditor/ckeditor5-engine/issues/1663 Range, Position, Marker and perhaps something else will still miss this method. Let's add it.

Also, lets remove as many instanceof checks from packages other than engine.

One remaining thing to check would be how often we use instanceof to distinguish between model and view things. Perhaps we could also have is( 'view:element' ) and secure our code a bit with this.

[msamsel]

Also, lets remove as many instanceof checks from packages other than engine.

I search through CKE5 repositories and I couldn't find any relations outside of engine package. Do you have some example where it is used? Maybe I make wrong search query.

[jodator]

@msamsel some of them were removed already or doesn't really existed for view entries. You can double check it with imports from engine - they shouldn't be necessary for affected elements.

Ie I've only see imports of Observers (separate issue),

[msamsel]

Thx @jodator for the hint. So I make double check for the import to have sure that haven't missed anything.

[Reinmar]

One remaining thing to check would be how often we use instanceof to distinguish between model and view things. Perhaps we could also have is( 'view:element' ) and secure our code a bit with this.

What do you think about this? cc @pjasiun

[msamsel]

Perhaps we could also have is( 'view:element' ) and secure our code a bit with this.

I've already make this change as it was quite simple and I already work with related code.

[scofalik]

Ugh, no. If you don't know if you are dealing with the view or with the model you did something horribly wrong.

Is there any use case for this? I can't imagine...

[msamsel]

if you are dealing with the view or with the model you did something horribly wrong.

It's quite good point. I haven't thought about it from such perspective.

[Reinmar]

@scofalik, that may be a part of some method's input validation. That you check whether the input are the proper positions. And if you'd do is( 'position' ) then it will accept both positions – from the model and the view. And this is actually a quite common issue that people use the wrong type of arguments and also quite tough to find, because e.g. view and model ranges have both start and end properties and couple others too.

[scofalik]

We almost never did this kind of validation and simply disregarded issues when people use wrong parameters in functions. I still think this is not needed.

[Reinmar]

Really? I would love if e.g. all writer methods were secured. And perhaps a lot of conversion and other plugin-developer-facing things.

Also, I'm really unsure if we can say that we're not doing this yet. Whenever we use instanceof now we're actually checking the type (model/view). If we'll replace those things with is() then we lose that.

However, we don't plan to change the engine (the only place where we use instanceofs), so I'm fine with that.