__ function returns Phrase object. Could this return string?

[apiuser]

Feature request from bentideswell, posted on GitHub Jun 22, 2016

The function returns a \Magento\Framework\Phrase object that when echo'd, is converted to a string via the toString method. This breaks several integrations I am working on and also doesn't work properly with json_encode (its saved as an object rather than a string).

Is it possible to have have this method return a string? The majority of time this method is used, it will be echo'd straight away, meaning the object isn't required anyway. A second function could be created that returns the object and the __ function could call this second function and cast the result to a string.

Here is a rough example:

function __O()
{
    $argc = func_get_args();

    $text = array_shift($argc);
    if (!empty($argc) && is_array($argc[0])) {
        $argc = $argc[0];
    }

    return new \Magento\Framework\Phrase($text, $argc);
}

function __()
{
    return (string)__(func_get_args())
}

This is needed as the __ function cannot be overwritten or extended like all other publicly available methods in Magento.

[apiuser]

Comment from adragus-inviqa, posted on GitHub Jun 22, 2016

I don't mind the request, but I'm guessing changing the return value of __() would break things.

[apiuser]

Comment from bentideswell, posted on GitHub Jun 23, 2016

I have done some limited testing and I have been unable to find an occurence where the object that __ returns isn't cast into a string. By using grep, I have searched for occurences of the function that are printed straight out and occurences that are assigned to a variable. Of the latter, these are always assigned to a variable with the word 'title' or 'label' in and are always then printed to the screen.

[apiuser]

Comment from Vinai, posted on GitHub Jun 23, 2016

One exception is the \Magento\Framework\Exception\LocalizedException, which takes a Phrase as a constructor dependency.
It even exposes Phrase internals like the arguments. The question is if that is really needed functionality. I would also much prefer to be dealing with plain strings, as having the Phrase in there just adds another dependency to all tests for code using __().
I find myself not using the LocalizedException for that reason.

[apiuser]

Comment from bentideswell, posted on GitHub Jun 23, 2016

Thanks for spotting that Vinai, I missed that one.

Would it be possible to modify the LocalizedException so that a plain string is passed in the constructor rather than a Phrase object. The constructor could then use the plain string to instantiate a Phrase object and store it is an protected variable of the class. This gives the LocalizedException class access to the Phrase object while allowing the __ function to return a string.

[apiuser]

Comment from Vinai, posted on GitHub Jun 23, 2016

Passing in a string would not give it access to the Phrase arguments, so that refactoring would not really be possible without changing the interface of the LocalizedException class.
There are several non-test places in the core where the methods getParameters(), getRawMessage() and getLogMessage() are called. So it's not that simple sadly. The exception class has an @api tag which means that change would require a major version bump. Maybe the core team (or a PR) can decouple the exception rendering though, and then the Phrase returned by __() could become a string. It would be cool if that change could then be scheduled to be included in the next major version.

[apiuser]

Comment from bentideswell, posted on GitHub Jun 23, 2016

Agreed, it would be awesome if it could be implemented as it's currently stopping me porting a few extensions to M2. I will look into alternative ways of doing what I need to do and hopefully the core team can look at this.

[apiuser]

Comment from vkublytskyi, posted on GitHub Jul 15, 2016

@bentideswell, would you provide more information what exactly issues you have in your integrations? Also please provide some code or steps to reproduce that

doesn't work properly with json_encode (its saved as an object rather than a string)

Magento\Framework\Phrase implements JsonSerializable interface so it is represented as a string in json_encode output.

As changing return type of __ function is a backward incompatible change we should have very strong reasons to do that.

---

Regarding LocalizedException. We acknowledged that it is not well-designed and general approach of it usage have drawbacks so we will review it in a near future.

[apiuser]

Comment from bentideswell, posted on GitHub Jul 15, 2016

@vkublytskyi, thank you for your reply.

The main issue I have is related to my Magento WordPress Integration extension. Magento and WordPress both use the function as a translation function. I get around this issue by wrapping the WordPress function with a call to function_exists and only allow the declaration of the function if it hasn't already been defined (by Magento). This worked perfectly and when integrated, WordPress relied on the Magento translation system to translate strings. This was acceptable as my extension also ported the WordPress translation file to a valid Magento translation file so translation could still occur in both Magento and WordPress.

In M2 I tried the same approach and the function was correctly not declared by WordPress and the M2 function was used. This triggered errors in WordPress though as a lot of WordPress core code uses the __ function around array keys.

Consider the following code:

$archives = array();
$archives[ __('January') ] = '01';
$archives[ __('February') ] = '02';

The above code is an example but there's multiple instances of similar code scattered through the WordPress core code.

In M1, this code worked fine as __ returned a string. In M2, an object is returned and an object cannot be used as an array key. Unfortunately, PHP doesn't automatically try to convert the object to a string, which would resolve the issue.

Hopefully this explains the issue fully but if not, feel free to ask as many questions as you need.

As for the json_encode issue, I may have made a mistake there and I haven't tested it as much as I should. That was more of a side issue for me anyway, with the main issue being the WordPress integration issue.

[apiuser]

Comment from vkublytskyi, posted on GitHub Jul 15, 2016

@bentideswell, thank you for explanation. We will investigate this use case to find solution or a work around.

[FishPig]

Do you have an update on this? This is getting very urgent for me. I cannot port several very popular Magento 1 extensions to Magento 2 until this change is made.