Still on Drupal 7? Security support for Drupal 7 ended on 5 January 2025. Please visit our Drupal 7 End of Life resources page to review all of your options.
Problem/Motivation
Currently, CKEditor5PluginElementsSubsetInterface
extends CKEditor5PluginConfigurableInterface
. I think the intention of this is to convey that returning an element subset only makes sense if the plugin is configurable.
However, to me that seems like an aspect of implementation. From the perspective of an interface, I'm not imagining a case where a caller of the interface or something that typehints to the interface would need to rely on anything in CKEditor5PluginConfigurableInterface.
Steps to reproduce
Proposed resolution
I'm leaning towards thinking that the interface shouldn't extend, but I'm open to arguments on why it should.
Comments
Comment #2
wim leersIf it's not configurable, how could it possibly determine what the relevant subset is if the signature is
?
Based on what input would it determine what the relevant element subset is? There's no input to the method, so it'd have to be hardcoded. And if it's hardcoded, then … the plugin definition never should have claimed a bigger set of elements is supported if it's hardcoding a subset of those elements?
Comment #3
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI agree with #2, but that's the perspective of the implementation. An implementor of
CKEditor5PluginElementsSubsetInterface
should also implementCKEditor5PluginConfigurableInterface
. However, the Interface segregation principle is about taking the perspective of the clients of the interface. Do the callers ofgetElementsSubset()
care about what other methods the object needs in order to implement that method in a sensible way?Comment #4
wim leersI see your point.
I don't see the harm in the current approach.
I guess we could remove that
extends
, but it's just mean nonsensical implementations are possible. That'd be worse for DX I think? I agree with the principle of composable interfaces. But when certain compositions do not make sense, what's the value in supporting them just for purist sake?IOW: I don't see the value in the proposed approach. Not yet, that is 🤓
Also, what are reasonable clients of this interface (aka callers)? The only sensible client is the CKEditor 5 module itself, isn't it?
Comment #5
bnjmnmI'm currently +! with @Wim Leers. I acknowledge the motivation behind wanting to make the interfaces independent, but I think the practical benefits of keeping the
extends
outweigh the conceptual benefits of removing it.There's a steep learning curve in developing for CKEditor 5, and I support leveraging any opportunity where a bit of opinionated code guides the developer towards steps that will ultimately be necessary.
CKEditor5PluginElementsSubsetInterface
extendingCKEditor5PluginConfigurableInterface
saliently enforces (via errors/IDE) the addition of methods that will inevitably need to be there anyway. Not doing this removes the benefits of immediacy and turns this into something a that adds to the already strained cognitive load of a developer figuring out how to extend CKEditor 5 in Drupal.There's also plenty of classes extending
FormInterface
that have emptysubmitForm
methods, and I think that's fairly uncontroversial. I certainly appreciated it when I was learning the poorly documented Drupal 8 API.I don't want anyone ordering soup to have to explicitly ask for a spoon. It adds inconvenience (and potential frustration) to request an item that is almost certainly necessary. While it's theoretically possible the customer might want to drink from the soup bowl, etc. those customers aren't offended by the presence of the spoon. However, customers being told they needed to request the spoon will almost always think less of the restaurant.
Comment #6
wim leersI'd almost RTBC this based on @bnjmnm's hilarious and surprisingly fitting metaphors alone 😆
All silliness aside, curious what you think, @effulgentsia! 😊
Comment #7
wim leersComment #8
wim leersComment #9
wim leersBack to @effulgentsia for review.
Comment #11
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI'm not sure I fully agree with, but I can accept, the premise that if there is no sensible way to implement CKEditor5PluginElementsSubsetInterface without also implementing CKEditor5PluginConfigurableInterface, that there is some conceptual benefit to, and no functional drawback to, having the former extend the latter. Therefore, closing this as works as designed. Thanks for considering it!