Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translation helpers should return the ID for untranslated messages as stated #1559

Open
albe opened this issue Apr 25, 2019 · 0 comments

Comments

1 participant
@albe
Copy link
Member

commented Apr 25, 2019

Currently, the EEL TranslationHelper states in its methods:

@return string Translated label or source label / ID key

However, it will instead return null for messages not translated, because the underlying Translator will return null for non existing messages translated by id:

@return string Translated message or NULL on failure

while it will indeed return the original label, if translated by label:

@return string Translated $originalLabel or $originalLabel itself on failure

This can be considered at least inconsistent and/or false code documentation.

Generally, it would be nice to get the translation ID as output for missing translations (at least in development context?) instead of an empty string (which null will eventually resolve to when rendered), because the ID is easier to see as a missing translation and immediately directs at the correct spot.

These are the options to go about this that I see:

  • fix the docblock in TranslationHelper to state it will return null for missing translations, keeping things as they are (a lot of custom translation helpers have been built that work around this shortcoming)

  • fix only the TranslationHelper by conditionally returning the ID if the result of the ->translate() call is null (See #1114) - this is breaking in a way that is probably hard to detect, because the output of this EEL helper will just change. OTOH a rendered translation key is probably as obvious as it gets.

  • fix the underlying Translator::translateById to be more consistent generally in how the Translation Framework works from the bottom up, in that a missing translation will return the translation input. This is more breaking, because it actually affects an @api method. OTOH, it will make the return type of this method stricter. This would mean that any checks for null to detect missing translations programatically will need to be changed to check for output === id, which is not coverable by a code migration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.