silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 821 forks source link

RFC Move Extension/Extensible into their own framework-agnostic package #10727

Open GuySartorelli opened 1 year ago

GuySartorelli commented 1 year ago

Motivation

Struck out as it seems it was distracting from the point.

I've recently been working on a set of tools written using symfony/console with the intention of lowering the bar for entry for new Silverstripe developers by providing a dockerised apache-based local development environment.

The more I work on this, the clearer it becomes that it won't be able to (and shouldn't try to) cater to everyone's needs out of the box - but a plugin system could allow people to add on any features or infrastructure they might need.

I could write a whole plugin system from scratch or find one out there and adopt it to my needs, and to some extent I'll need to do that, but if the extensions API was its own framework-agnostic package I could use it as the backbone for my plugin system. This tool is for silverstripe developers, so being able to provide an extension API they're already familiar with will make it a lot easier for people to make plugins for this thing.

Examples

Concept

The extensions API is primarily driven by the following classes and traits:

Of these, the only coupling with framework is in the Extensible class, and is as follows:

This functionality also relies on the Configuration API provided by silverstripe/config which itself was decoupled from Framework until very recently and will take very little effort to decouple again - see https://github.com/silverstripe/silverstripe-config/pull/82

And before anyone says it, I don't think we should move any subclasses of Extension such as DataExtension - those belong in framework, as they're providing the method signatures for framework-specific extension hooks.

Namespaces

Because this proposal comes so late after the CMS 5 beta release, I suggest we don't change the namespaces for now - instead we lift-and-shift the relevent code into the new package, add it as a dependency of silverstripe/framework, and remove the code from framework.

Note that with this approach we could do this during a minor release if we wanted to (the API wouldn't actually change) - but I propose we try and get it done before the CMS 5 stable release and mark the new package as a supported module.

We can then change the namespaces when we start working on the CMS 6 major release cycle.

Benefits:

Drawbacks:

michalkleiner commented 1 year ago

I'm even less sure about the examples here than the motivation in #10728, but if it's not much effort then possibly why not. Since I don't have a view of the whole CMS5 and CMS6 roadmap (if there's any), it's hard for me to judge whether things like this and #10728 are what the community is mainly after, whether it's the best use of the squad resources. I'm not sure a flexible plugin system and docker env tooling is what is expected from the CMS at this stage, compared with e.g. working history viewer, less friction React enhancements/integration within the CMS and so on.

GuySartorelli commented 1 year ago

The plugin thing and the Dev env project are something I am personally doing in my own time and doesn't have anything to do with the priorities of the CMS squad. They were included here as an example of the sort of project that could benefit from this extension API that doesn't require the rest of framework. I'll cross that out as it seems to be distracting fron the point.

michalkleiner commented 1 year ago

Fair enough. I guess it didn't cross my mind to distinguish the work within the squad and personal projects, but it's of course possible to have both ✌️

emteknetnz commented 1 year ago

It's not clear to me what the main benefit of doing this is?

Is the primary intention here to: a) be able to use silverstripe/extensible as a standalone composer requirement on a non-silverstripe project? b) reduce the weight of framework?

For either of those I'm mostly negative a) There aren't any other core module that are intended to be used this way? The closet I can think of is silverstripe/recipe-core that was intended to be a non-cms install of silverstripe, though my understanding was that it was never really popular and everyone just used silverstripe/recipe-cms via silverstripe/installer. I'm not really a fan of creating a new "product" to "support", if that makes sense b) As a maintainer I don't see any benefit whatsoever in having silverstripe/config separate. In fact it's probably more the opposite, it's a slight annoyance having another module the maintain separately, when it practice it's a part of silverstripe/framework (there may be a valid reason why it's separate, but easier maintainability isn't part of that reason IMO)

GuySartorelli commented 1 year ago

The primary intention is a) be able to use silverstripe/extensible as a standalone composer requirement on a non-silverstripe project

There aren't any other core module that are intended to be used this way?

silverstripe/config started out as its own component that could be used this way, and it still could be until we very recently erroneously coupled it to framework. There's a PR to decouple it again. But even if we ignore that as precedent, "we haven't done it before" is not a good reason to not do something like this IMO. This takes existing code that we already support and maintain and just shifts it into its own package where literally the same code can now be reused easily without requiring the entire framework as a dependency.

I don't think we'd be increasing our maintenance burden by doing this, as we're already maintaining this code. If anything, we reduce our maintenance burden by making it easier to isolate any issues related to this part of the code.

michalkleiner commented 1 year ago

Having to have another module/repo is an increase in maintenance on its own. You need to think about a number of other things related to having a new module, e.g. translations, CI runs, does it have correct issue labels created, branch management, version tagging etc. I'm not saying that should preclude us from having smaller packages, it's just not a zero increase.

Back in the day either Sam or Hamish talked about splitting the CMS and the framework into a bunch of smaller modules, which this would be in line with, but probably needs a broader consensus and mid- to long-term plan on what those packages would be etc. If they were locked together via major versions then I'm not sure the benefits to spend the time on that are there.

At the same time I would support being a tad more progressive with new features and moving faster, I don't want to come across as completely dismissive.

maxime-rainville commented 1 year ago

Personally, I tend to think of modularity as something that is inherently good. Basically, by having a hard separation between things that don't necessarily need to be together, you force yourself to avoid creating interdependencies ... or if they are interdependencies, you highlight them explicitly.

It's not obviously clear to be me that there's a massive overhead between managing a lot of smaller modules versus managing one big one. If I was starting from scratch, my inclination would be to spilt framework up into a bunch of smaller components.

However, we are not starting from scratch, so we should consider that. I don't necessarily see this as a super valuable change. If there's a way to do this without breaking APIs and there's not a substantial amount of work needed, I'd be OK doing this kind in CMS5.

Beyond that, I think it could be valuable to look at splitting framework up in CMS6 more consistently to have a better architecture and look at decoupling things.