@mpdude I just thought about another case where TranslatorBagInterface could be useful:https://github.com/willdurand/BazingaJsTranslationBundle/ is exposing translation catalogues to the JS code. Currently, it does that by reimplementing the loading itself (it gets all translation loaders from Symfony and gets all resources as well, all of which is done by the Translator) because TranslatorBagInterface was not available at that time. What would be the clean way toimplement such your case in your new architecture (duplicating the loading is not the clean way in the 2.6 architecture IMO, but it was probably the only way in the 2.3 architecture, which is why it is used)

well I think we need to keepgetCatalogue and the optimize caching should be done by optimizingMessageCatalogue see#14526

@stof In the case of messages available in both the primary and in fallback catalogues, which set of messages do you think this bundle would need?

The problem is that if we burdenTranslator with general tasks like providing completeMessageCatalogue chains, it gets harder to optimize the data structures used internally for the "find best translation available" use case (thetrans() andtransChoice() methods).

We could, of course, internally use optimized structures for these methods plus a "complete" set ofMessageCatalogue chains for theTranslatorBagInterface-based use cases, but that would clearly be a nightmare to maintain and would not make a good example for the SRP.

So, what if we had two different classes (or services in the case of the full-stack framework):

The first one is where the resources and loaders are registered. You tell it your preferred order of locales, it will return a full-fledgedMessageCatalogue chain (expensive operation!).

The second one is theTranslator. It will initially obtain theMessageCatalogue from the first class, but then optimize/tweak things as it sees fit for thetrans() andtransChoice() methods. These optimized structures can be kept in the cache and will not be exposed to the outside world.

@mpdude This bundle would need anything needed to be able to reimplementtrans andtransChoice in JS. So it would need the full catalogues for any active locales (it currently supports activating multiple locales, in case the locale choice need to be done on the JS side, or to keep a single translation file for all locales to simplify the loading of the JS), but only fallback messages might be enough for other locales.

And I agree that delegating all the loading to a separate class might be a better architecture. It could also simplify a bunch of things:

• lazy-loading of loaders could be implemented by implementing this interface in a lazy way (or relying of the DI feature to build the lazy implementation) instead of extending the Translator in FrameworkBundle, which would also make the lazy-loading available for people using DI+Translator outside Symfony
• caching of catalogues could be implemented by composition instead of being in the main class dealing with loaders (which does not forbid caching the internal structures of the Translator class, which is a separate topic).

This is something I already thought about in the past. The hard part is the Translator class is currently designed to be extendable through inheritance. Extracting this logic into separate collaborators in a BC way will likely be very hard.

@stof FYI I work on delegating all the loading :)

@aitboudad have you found a way to keep BC for people overwriting protected methods of the Translator to customize the Translator through its current extension points ?

@stof see an initial work forDelegationLoader I'll complete it soon, I think we need to deprecateSymfony\Bundle\FrameworkBundle\Translation\Translator.
To avoid BC we will create a new translator service and deprecate the default one.

@mpdude why we can't probably not move that responsibility out into another class ? until now
I have only one issue aboutinitializeCatalogue, I'll see how to deal with it.

@stof should we move handling fallback catalogues intoDelegationLoader or by composition instead ?

What is the status of this PR? ping@aitboudad

I rebased this branch and fixed the various issues from the first review by@aitboudad and@stof.

This won't take us to the ultimate goal of a better and more efficientTranslator (#13948) because we'd probably also need to factor loading out of theTranslator and prohibit inheritance (this andthat comment and maybe something like the#14622 or#14671 PRs?)

Anyway, this IMO improves our situation because we can (in 3.0) remove an interface that definetly is a show-stopper.

@fabpot ll try to finish#14622 today as I think it's more efficient than this one(ideally we should have something likehttps://github.com/openl10n/openl10n-translation/tree/master/src/Openl10n/Component/Translation/MessageCatalogue)

@symfony/deciders I would like to ask for your vote on this.

I believe that removing the TranslatorBagInterface is one important prerequisite for more efficient caching and/or loading inside the Translator because it improves encapsulation.

With the current state of this PR, one important thing to note is that we leave clients without a way to retrieve the message catalogue from the translator, a feature added in 2.6 "by accident" (it would not have been necessary for the feature we added back then).

To provide an alternate means of retrieving the catalogue is an on-going effort in#14622 and#14671, but with the internal structure ofTranslator (different code paths, mixing several concerns,protected methods and@api) I am unsure if we can make it.

So, the decision you need to make is whether the improvement this PR brings is worth this possible annoyance for users of theTranslatorBagInterface in case that no other replacement for it can be provided.

Note that@stofrefers to one third-party bundle that workswithout this interface by implementing the loading itself.

(The failing test is for PHP 7 is completely unrelated.)

I think we don't need this one: optimized caching is already dealt with on PHP7, where static arrays and static strings are far superior optimizing strategies. Thus, no need to do anything more on our side to me.

@mpdude any thoughts on what Nicolas said?

Closing as explained. Of course, please reopen if you don't agree.

I opened this a while ago when I was working on a project where we had issues due to a large number of languages supported and comprehensive catalogs.

We found other ways to work around this and I cannot tell whether the problem would still appear on PHP7.

So, closing this is fine for me. Will reopen it or make a new case should I run into it again.