[Translation][FrameworkBundle] Fix resource loading order inconsistency reported in #23034 #23057
Conversation
| * | ||
| * @var array | ||
| */ | ||
| private $otherResources = array(); |
There was a problem hiding this comment.
Yes, using this array to collect addResource() calls and actually execute them after the resource_files is ugly, but we don't have access to the private \Symfony\Component\Translation\Translator::$resources field so that we could "unshift" the resource_files in front of it in loadResources.
|
Is loading the resources immediately in the constructor (here) really necessary (why)? Commenting it out does at least not break tests. |
|
@fabot / @nicolas-grekas does this look sensible? If you basically agree, I probably should rebase it onto 2.7. |
| @@ -38,6 +38,14 @@ class Translator extends BaseTranslator implements WarmableInterface | |||
| private $resourceLocales; | |||
|
|
|||
| /** | |||
| * Holds parameters from addResource() calls so we can defer the acutal | |||
There was a problem hiding this comment.
typo: actual
let's rebase on 2.7 if it applies there
|
Rebased onto 2.7; no modifications were necessary |
|
Thank you @mpdude. |
…inconsistency reported in #23034 (mpdude) This PR was squashed before being merged into the 2.7 branch (closes #23057). Discussion ---------- [Translation][FrameworkBundle] Fix resource loading order inconsistency reported in #23034 | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23034 | License | MIT | Doc PR | Fixes the bug reported in #23034: When mixing `addResource()` calls and providing the `resource_files` option, the order in which resources are loaded depends on the `kernel.debug` setting and whether a cache is used. In particular, when several loaders provide translations for the same message, the one that "wins" may change between development and production mode. Commits ------- 2a9e65d [Translation][FrameworkBundle] Fix resource loading order inconsistency reported in #23034
|
My app has a custom loader to load translations from a REST service. This change broke it. It broke because I was injecting the translator into the loader and calling Now I suspect this is my error, injecting the translator into a loader certainly doesn't sound the right thing to do. So I was just after confirmation that this is indeed my issue and any tips on the best way to load resources outside of the loader would be greatly received too. I initially thought a compiler pass, but I don't think it's as simple as that? i.e. I couldn't call |
|
Note: I have now successfully used a compiler pass to work around this. Have opened a PR though to highlight the BC break, may decide it's not a use case we need to support but it's there for deciders anyway 👍 |
|
Creating a circular reference between the translator and the loader to have the loader reconfiguring the translator regarding what resources should be loaded by the loader is indeed a use case we never intended to support. |
Fixes the bug reported in #23034:
When mixing
addResource()calls and providing theresource_filesoption, the order in which resources are loaded depends on thekernel.debugsetting and whether a cache is used.In particular, when several loaders provide translations for the same message, the one that "wins" may change between development and production mode.