codefog / contao-haste

Haste is a collection of tools and classes to ease working with Contao
http://codefog.pl/extension/haste.html
MIT License
43 stars 24 forks source link

Keep `Haste\` root namespace for 5.x #177

Closed fritzmg closed 1 year ago

fritzmg commented 1 year ago

I see you are working on 5.x for Contao 5 compatibility :) 🎉 . However, I would suggest to leave the Haste\ root namespace instead of moving everything into Codefog\HasteBundle\. This way it is easier to keep extension versions compatible with both Contao 4 and Contao 5 (and thus also Haste 4.x and 5.x). I think it's perfectly reasonable to have FQCNs like Haste\HasteBundle and Haste\ContaoManager\Plugin etc.

Otherwise having to have many class_exists checks would be a little bit of a pain.

fritzmg commented 1 year ago

Although on the other hand, 5.x will still be compatible with Contao 4.13 anyway, so the extensions can upgrade their requirement to ^5.0 anyway (assuming they do not want to be compatible with 4.9 anymore).

aschempp commented 1 year ago

Yeah, a major version is desperately necessary, but it will be a pita to upgrade. We also discussed splitting the remaining features into new bundles so they could co-exist, but ultimately decided against it.

I honestly don't know how this is gonna work out 🙈

fritzmg commented 1 year ago

I at least upgraded one of my extensions to use 5.x-dev already - though I mostly always use the Form from Haste and not much else (may be a toggle operation here or there).

qzminski commented 1 year ago

I leave this decision up to you guys @aschempp @Toflar as you'll certainly know better what to do 😉

Toflar commented 1 year ago

Using Haste as your vendor namespace is wrong. The new one is correct so we should definitely stick with that. However, I completely agree that using Haste hasn't been an issue all these years and we should try to make it as easy as possible to developers to be compatible with the latest haste 4.x and 5.x in order to be compatible with Contao 4.13.x and 5.x.

I think the namespace issue can be addressed by a class_alias.php that we autoload which contains all class_alias() calls for all the classes that remain the same. This obviously only works if the namespace change is the only API break. That's why I would e.g. refrain from things like renaming methods on the Form class if they do not provide a real benefit.

For all the other cases, we can provide simple BC layers (even for Form if you want to rename the methods). E.g.

class StringUtil
{
    public static function foo(): string
    {
        return Contao\System::getContainer()->get('foo.service')->newFoo();
    }
}

This requires that foo.service is public but that should not be an issue.

I would place those files all in a folder called bc_layer or so and use classmap autoloading for that. In Haste 6, we can then simply drop this folder and the autoload definition.

Toflar commented 1 year ago

So after long discussions, we decided to settle with the new namespace and introduce hard breaks. The reason for it being mainly that it does not make sense to provide BC for the few things that remain in Haste. Haste 5 is going to be a very reduced set of helpers because version 4 almost pre-dates Composer. Today, you can choose all sorts of helper libraries from Packagist, you don't need Haste anymore for 90% of the things. Haste is still useful in mainly 3 areas:

For all the rest, there are better alternatives. Consequently, this means that you probably don't have too many calls to Haste itself anyway so making things compatible with Haste 5 should not be a big deal.

Haste 5 will be compatible with Contao ^4.13 || ^5.0 which means you can ensure compatibility of your extensions with both, Contao 4 and 5. You only need to do class_exists() magic, if you want to support versions < Contao 4.13 in which case, you need to support both, Haste 4 and 5.

We know that we're forcing people to update their extensions to only support 4.13 with this decision. But we think that's not a problem as 4.9 enters security-only support mode very soon. They cannot expect bugfixes in Contao anymore so they also don't need to expect bugfixes in extensions. If they need bugfixes, they have to upgrade to 4.13 in which case, your extension can rely on Haste 5 and be happily compatible.

Of course, if someone wants to provide some kind of haste-bc-package that mirrors old Haste 4 calls to the Haste 5 API, nobody is stopping you from doing that. But we're not going to put in the work into doing that and keep maintaining those BC layers.

So as of now, the 5.x branch is pretty much ready. We're having some discussions over some leftover utils but I think you can start with some test branches in your extensions that require Haste 5 for Contao 5 compatibility and see, how many adjustments are really needed.