-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Intl] Twig is not using bundled ICU data #15007
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
Comments
Can you run this code to see what you get ? <?php
$formatter = NumberFormatter::create('en_MY', NumberFormatter::CURRENCY);
var_dump($formatter->formatCurrency(100, 'MYR')); |
hmm, the Intl component ships with ICU 54.1 data. This may be the reason of the inconsistency between calls done based on the ICU data bundled inside PHP and the calls done based on the data bundled in the Intl component (because the intl extension does not give access to all ICU data). The currency might have changed in ICU between these versions. |
"MYR100.00" Yes, I believe the ICU data has changed. And just as #11887 I do think that Symfony/Twig can move away from PHP's Intl component completely (again, as mentioned before it's been shown to be quite possible with https://github.com/commerceguys/intl) |
Well, reimplementing the intl extension in PHP rather than using the C code is likely to be slower for the same task. so this would look weird. A benchmark comparing how and the Twig-extensions repo cannot rely on a Symfony component. Twig is totally independant of Symfony. |
Symfony 2.3-dev on a system with ICU 55.1:
Symfony 2.3-dev on a system with ICU 52.1:
Symfony 2.3.36 on a system with ICU 55.1:
Symfony 2.3.36 on a system with ICU 52.1:
|
Twig will always use the ICU data installed on the system to format the currency. However, getting the currency symbol alone is not exposed through the intl extension, so Symfony needs to load it from ICU data bundled with the intl component. We'll have the same issue when using the intl extension classes directly if the ICU version installed on the system is different to the one bundled with the intl component.
There's not much we can do about it, apart from keeping ICU data bundled with the intl component up to date. End users should keep their system versions up to date as well. The correct version is always the latest released one. The issue started as "Twig is not using bundled ICU data". I think we can close it as twig we won't make twig rely on Symfony. We could potentially warn users if they have a version mismatch (something for SE's check.php?), and add a warning in the documentation (if it's not there yet). |
Closing for the reasons given by @jakzal. Thank you very much for this amazingly detailed analysis! I've opened https://github.com/symfony/symfony-standard/issues/952 to consider adding that warning message when there is an ICU version mismatch. Thanks! |
Hi guys. I got here from symfony's
What should one do? What commands or scripts to run to see the difference in versions and understand what action might be required? Spent about 30 minutes on a research, but with no results. |
@kachkaev I think the current message is confusing, could you please have a look at sensiolabs/SensioDistributionBundle#274 to see if it's any better? I'd appreciate some feedback. |
This PR was merged into the 5.0.x-dev branch. Discussion ---------- Make the icu data version check less confusing People [get](symfony/symfony#19453) [confused](symfony/symfony#15007 (comment)) by the current message added in #262. It only makes sense to be on the latest ICU data, as any other distribution is invalid (outdated). Therefore I suggest we keep the icu data up to date (symfony/symfony#19468 was just merged), and raise a notice if the user is on an older version. Commits ------- 67713ce Make the icu data version check less confusing
I have a problem installing symfony 3.1 in php7, nginx and ubuntu 16.04, i have this error: intl ICU version installed on your system is outdated (55.1) and does not match the ICU data bundled with Symfony (57.1) To get the latest internationalization data upgrade the ICU system package and the intl PHP extension. |
Hi, I have the same error @ahmed0406 ... |
It is exactly as the warning message suggests. You need to upgrade the ICU package on your system. But whether such an updated package is available depends on your OS / distro. You might get some data mismatch if you ignore this warning. Not really a big deal for most users, I suppose. |
|
Of course this is perfectly understandable, but it creates inconsistencies in the results...
e.g.
vs
with ICU 52 (Ubuntu 14.04 "trusty")
The text was updated successfully, but these errors were encountered: