Twig intl-extra depends on intl PHP extension, but does not declare such dependency

[urbanecm]

twig/intl-extra depends on intl PHP extension. However, it is possible to install even on systems without that extension (composer install succeeds). In that case, its usage would end up with a quite mysterious error:

PHP Fatal error:  Uncaught Error: Class "IntlDateFormatter" not found

As https://www.php.net/manual/en/class.intldateformatter.php says, the IntlDateFormatter class is defined by the extension.

composer.json supports declaring dependencies on PHP extensions as well (by depending on ext-intl). I propose we add that dependency, which would make composer.json complain directly when installing.

Maybe the goal is to provide a fallback implementation somewhere (https://github.com/symfony/polyfill-intl-icu/tree/1.x?tab=readme-ov-file seems to be providing it, although that package is deprecated), but it doesn't seem intended to fail on usage. Whether the expected behaviour is "fail on install" or "use a fallback", I'm not sure.

[stof]

although that package is deprecated

Where did you see that this package is deprecated ?

[ericmorand]

composer.json supports declaring dependencies on PHP extensions as well (by depending on ext-intl). I propose we add that dependency, which would make composer.json complain directly when installing.

Wouldn't that make the Twig extension impossible to install on systems where ext-intl is not available? The important word is install. That the Twig extension can't be executed on system without ext-intl is one thing, and a legit one. But what you propose is that it can't be installed. This is way different. When we install a project, we don't need to have a runtime environment that matches the runtime constraints of the dependencies, since we only need the depenencies to be downloaded.

Where the produced artefact is ultimately executed is out of the scope of composer.

[xabbuh]

When we install a project, we don't need to have a runtime environment that matches the runtime constraints of the dependencies, since we only need the depenencies to be downloaded.

If you don't care about your environment being able to actually run the code, you can instruct Composer to ignore them with the --ignore-platform-reqs option.

[nicolas-grekas]

Well, that doesn't really work in this case. Take eg Drupal, which could require this package and decide to attach the polyfill to make installing it easier. If we'd add ext-intl, it wouldn't make sense for them to tell their users to run composer with the --ignore-platform-reqs option.

[stof]

@ericmorand see my comment on the PR: #4703 (comment)

The extension can run with either ext-intl or symfony/polyfill-intl-icu, not just with ext-intl.
and symfony/polyfill-intl-icu cannot declare it provides ext-intl (unlike some other symfony polyfill packages) as it is a partial polyfill:

• it supports only the en locale (but its polyfill for Locale::getDefault() also returns en, making the simple usage relying on the current default locale working)
• it polyfills only parts of the ext-intl API (we have some other symfony/polyfill-intl-* packages covering some other parts, some of them being big due to embedded some data about Unicode for instance)

[urbanecm]

although that package is deprecated

Where did you see that this package is deprecated ?

I was looking at https://symfony.com/doc/3.x/components/polyfill_intl_icu.html. However, on second look, it is actually saying that the documentation might be out of date. I arrived to that page via googling – for some reason, when I searched for symfony/polyfill-intl-icu, this link was one of the first ones.

That being said, the link from "the updated version of this page" points to https://github.com/symfony/polyfill_intl-icu, which is a 404. So, this misguided me as well (since it links to GitHub, I assumed the banner is talking about codebase).

[urbanecm]

composer.json supports declaring dependencies on PHP extensions as well (by depending on ext-intl). I propose we add that dependency, which would make composer.json complain directly when installing.

Wouldn't that make the Twig extension impossible to install on systems where ext-intl is not available?

Why would we need to install the intl extension of Twig on a system where ext-intl is not available?

I can see that some projects might want to make the Twig extension an optional dependency (and are able to use it if it exists, but don't strictly require it). In that case, I think they probably shouldn't list the optional dependency under require, but under suggest (docs. That would avoid the need for the code to be installed.

I can also see some testing needs ("what does this do if the extension is actually missing"). For that, --ignore-platform-reqs seems to be an usable tool.

Also: If we really want to allow installing without ext-intl (by default), then I'd say the extension should check the condition on runtime, and throw a more meaningful error than "class not found". I'm happy to follow-up on the PR to make it do that instead if that sounds like a good idea?

[urbanecm]

Well, that doesn't really work in this case. Take eg Drupal, which could require this package and decide to attach the polyfill to make installing it easier. If we'd add ext-intl, it wouldn't make sense for them to tell their users to run composer with the --ignore-platform-reqs option.

I'm not sure I understand you/polyfills correctly, but for the polyfill to be useful, wouldn't it need to be downloaded together with the extension as well (so, a dependency between twig/intl-extra and the polyfill)? Or am I missing something here?

[ericmorand]

@xabbuh

If you don't care about your environment being able to actually run the code, you can instruct Composer to ignore them with the --ignore-platform-reqs option.

I know, and this is what we already have to do for every single project that we are working on: documenting that the project needs to be installed using composer install --ignore-platform-reqs, which is a bummer because we would prefer that developers just have to use composer install.

There are various way to have a runtime constraint satisfied at runtime, and composer has no way to know these ways at project install time - this is strictly impossible. As mentioned by @nicolas-grekas, Drupal uses one of this way to provide intl at runtime, and forcing everybody to use --ignore-platform-reqs means that it should be the default - which consequently means that those runtime constraints are useless since everybody is expected to ignore them.

Nowadays, we should try and remove these constraints instead of adding more of them. We have been living, for more than a decade now, in a world where the machine where the project is installed is rarely the one where the artefact is executed. Even tests are run in sandboxes that satisfy all the requirements of the artefact - a container.

@urbanecm

then I'd say the extension should check the condition on runtime, and throw a more meaningful error than "class not found"

This I agree 100%.

[stof]

I'm not sure I understand you/polyfills correctly, but for the polyfill to be useful, wouldn't it need to be downloaded together with the extension as well (so, a dependency between twig/intl-extra and the polyfill)? Or am I missing something here?

this would mean that all projects that actually require ext-intl (because they expect to be able to use full localization features) would still end up downloading symfony/polyfill-intl-icu, while never needing to use it.

What twig/intl-extra actually requires is having ext-intl OR symfony/polyfill-intl-icu, but there is no way to define that in composer requirements (with userland packages, that can be achieved with virtual packages provided by each alternative, but there is no way to tell composer that a virtual package could be provided by an extension).

I can see that some projects might want to make the Twig extension an optional dependency (and are able to use it if it exists, but don't strictly require it). In that case, I think they probably shouldn't list the optional dependency under require, but under suggest (docs. That would avoid the need for the code to be installed.

Making the extension optional means they would have to guard its usage in Twig template, with a fallback to a different formatting filter. That's a lot more painful that using an English-only polyfill of the formatter allowing to always use the localization-aware Twig filter while being compatible with projects not caring about localization.
Note that the target for such use case is not end project controlling their production setup. It is shared libraries or frameworks (like Drupal) that must cover different use cases.

That being said, the link from "the updated version of this page" points to https://github.com/symfony/polyfill_intl-icu, which is a 404. So, this misguided me as well (since it links to GitHub, I assumed the banner is talking about codebase).

I submitted a PR at symfony/symfony-docs#21537 to fix the broken link for the repository.