With implementation of Dependency Injection of Symfony we can use
\Vendor\Ext\OriginalClass:
alias: \Vendor\AnotherExt\OverwritingClass
instead of XCLASSing with following hook:
$GLOBALS['TYPO3_CONF_VARS']['SYS']['Objects'][$classToOverride] = [
'className' => $overwritingClassName
];
Do we want to keep XCLASS hook or switch over to new Symfony DI?
Deprecate XCLASS for TYPO3 11 or 12?
Hey @froemken,
I’m fine with deprecating this in v11, and removing it v12. This would mean: One less thing to worry about using GeneralUtility::makeInstance()
From personal experience, DI is still quite clunky (or at least not well documented) as opposed to Xclass which is a well defined feature and way to do stuff but I’m all for moving forward and after losing most hooks in favor of PSR-14, I don’t see why this should not be the next big step.
A thing that extension developers will surely appreciate is a script that creates the conversion from $GLOBALS['TYPO3_CONF_VARS']['SYS']['Objects'] to Symfony DI alias, so as long as something like this is provided, I’m all for it.
Please deprecate and trash it ASAP.
Basically, I vote no unless there is a 100% alternative that covers every (yes, every!) case that XCLASS can currently cover; I don’t really care about which version. It’s not enough to have a script that would migrate from XCLASS to DI, it also must be able to support XCLASS’ing classes from 3rd party extensions that did not declare their classes in DI, and it must also be possible to do for classes that are not declared public in DI.
Okay. Then just deprecate XCLASS without “version deadline”. People should just stop using it if possible and should feel guilty if they use XCLASS.
Then just deprecate XCLASS without “version deadline”.
Sort of agree, but only if there’s at least a chance of finding a replacement that covers 100% of the XCLASS use case.
People should just stop using it if possible
Yes, but the point is that it isn’t always possible to stop using it. This is true about 3rd party code as well as core implementations.
and should feel guilty if they use XCLASS.
Absolutely not! You should never feel guilty about having to use XCLASS - the very reason why it exists and is still being used, is precisely that it covers use cases that no other functionality can cover; those edge cases where you’ve got zero other chances of replacing an implementation.
Hey, thanks for bringing this up.
I think we need to differentiate between services and data objects.
So, we provide XCLASSES for both types, service classes (controllers, repositories, utilities…) and for data objects (e.g domain models).
On the one hand our service management improved alot via Services.yaml/php configuration and while we may discuss documentation, I think yes, service configuration is the right place for service overrides and a better, non global and side-effect free approach than XCLASSES.
I have to point out a disclaimer though, service overrides should ideally be implemented as decorators, or should at least not overwrite the constructor (as constructor parameters may change for new dependencies).
…but I didn’t test https://symfony.com/doc/5.2/service_container/service_decoration.html with our symfony DI integration yet, as that feature was not available when I integrated symfony 4.x DI.
It’s actually important to rather use decorators instead of simply exending classes as our DI constructors are thus likely to break overwritten services during updates (e.g. think of security updates that require new dependencies for some services), otherwise users don’t really gain anything by migrating away from XCLASSES, besides syntax sugar.
And just to stress that this is still the “right” thing to use the service configuration:
service overrides are also considered in the service-provider container-interop working draft: https://github.com/container-interop/service-provider#entry-overriding (providing support for both, full overriding and full decoration).
Therefore yes, I fully agree that we should officially recommend to configure service overrides via Service.yaml/php configuration and should deprecate XCLASSES in v11, but we should not recommend aliasing, but rather decoration of services (mostly interface implementations) if possible, and only otherwise aliases…
On the other hand we do not have an nice alternative to XCLASSES for extending/overwriting data object classes, and I actually don’t see a nice alternative. We just removed the extbase-native way and rather recommended to use XCLASSES: https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/10.4/Deprecation-90803-DeprecationOfObjectManagergetInExtbaseContext.html
While I personally try to avoid XCLASSES for data objects (rather use composer patches, which fail during composer update, rather than runtime), I see the usecase for some and therefore would really actually leave this as “feature” for data objects.
To summarise:
I agree to deprecate XCLASSES for services, but would like to recommend/educate a more solid approach decorator approach.
I thus recommend to throw a deprecation if an XCLASS is configured for a Service (technically that means only in GeneralUtility::makeInstanceForDi – which is called when a service is instantiated via the container).
I would not throw a deprecation for data objects, and simply leave xclasses as in for regular GeneralUtility::makeInstance().
Note, this goes in line with future plans to make less and less use of GeneralUtility::makeInstance() for Services/Singletons in favor of dependency injection, in order to really only use GeneralUtility::makeInstance() for new data instances and to longer misuse that as service/singleton container. Therefore its good if we now separate class overrides for services and data objects into two different mechanisms.
Couldn’t that follow the same approach as with Services? What classes do actually create the data objects? Shouldn’t that be services? Shouldn’t it therefore be possible to decorate the methods of services to call parent, get original data object, and create an adjusted new data object out of it if necessary?
That would require well written code, but I guess that was even more necessary for xclasses.
I agree with @bfr that service objects and data objects are different creatures and should be considered separately. I also agree that the correct answer for service objects is “use the container and constructor injection, move along.” The sooner that gets deprecated and removed, the better.
However, I don’t agree that there’s a need to keep XCLASS for data objects.
For one, a data object should be a value object, with no dependencies. It’s purely data, so there’s no dependency injection to worry about.
If you need to have a mutable object of some kind that is service-aware, that’s not a data object but a command object of some variety. (Even if it’s self-executing.) Those can and should be spawned off of the relevant service, as @layne.obserdia1 notes.
Two, if you really need to provide an alternate version of a data object, then you can subclass the object and use that, so it’s still type-compatible. If it’s realistic to have multiple variants of the same data object, that’s what interfaces are for.
Making it possible to sideways-hack into any other module to mess with its classes in arbitrary ways acts as a crutch, in my experience, that robs you of the incentive and insight to build more robust, extensible code in the first place based on seeing what types of modification really are necessary and appropriate. (Those are two separate things.)
As far as timeline, I’m flexible, but note that the syntax for constructor DI is going to be much much nicer in PHP 8.0 thanks to constructor promotion, and v12 is most likely going to have a min version of at least 8.0 or higher. So it may be prudent to save any large overhauls in that department until then as it will be a lot more pleasant to work with. (Not that I want to dissuade anyone from working on it before that.)
One of the strengths of TYPO3 is its extendability and we should really ensure, that the possibility to extend TYPO3 and 3rd party extensions stay at the same level as current.
In TER, we have currently 143 extensions (since 2016) which use XCLASS to extend TYPO3. So if we deprecate/remove XCLASS, we must provide a good documentation for developers that covers all possible migration scenarios.
Also we should (independently if XCLASS will be deprecated or not) update the official TYPO3 documentation with best practices on what is the recommended way to extend TYPO3 / 3rd party extensions.
Examples:
I’m in to deprecate/remove XCLASS for scenarios, where we can provide an alternative. But for scenarios, where XCLASS is the only possibility to extend TYPO3 / a 3rd party extension, we should keep it as it is.
I am also in favor of removing XCLASS.
Yet I would not market DI alias as a good replacement.
In order to facilitate debugging and user support it should be really obvious if Core or extensions got changed. Currently that can easily go under the radar of an integrator. This leads to non-tracable error reports.
There should be real patches during composer install stage and if an extension wants a Core change it would have to note how to configure the root composer.json for that. This is a solution for all extensions using XCLASS right now.
That said, I would also very much welcome deprecating non-composer installs.