-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Add a Composer hook to compress + optionally remove locales not in framework.enabled_locales #51443
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
The hook is easy, as Composer allows running any command as hook: {
"scripts": {
"post-install-cmd": [
"php ./vendor/symfony/intl/Resources/bin/compress"
]
}
} |
regarding to removing languages not enabled in the FrameworkBundle config, this cannot be done in a standalone script provided by the component, as it has no knowledge of the kernel of your application. |
@stof it might make sense to then read the locales from composer extra, like the given example does. Although it would be better if this was integrated in some way. |
note application enabled locales and available intl locales are separate lists, they dont correlate per se |
@ro0NL how so, why would I want some locale in intl which is not enabled in the app? |
because the intl data can be used by other means, regardless of the locales your app supports for its own i18n purposes |
In what ways exactly? |
another issue is form integration, an app might use the |
In which case you don't opt into the cleaning, of course. |
sure, the issue arises when it creates 2 competing usecases eg. once you need all intl locales after starting with a filtered list. Then you suddenty need to account for filtering within your app it sounds like a possible footgun :) for what is IMHO little gain (ie. diskspace) |
Disk space is not little gain. |
fair :) im just not sure it's worth it, since it can be defeated at any time (eg. as soon as any locale but an 'app enabled one' is needed) |
Yes, but since the code which can use an arbitrary locale is limited and known, it's not like it will spontaneously spawn in your app at runtime (and hopefully you don't execute user provided code directly). If you're using arbitrary locales, don't opt into the cleaning. If you're not using them, why wouldn't you want to clear a bunch of unused locales and reduce your install size? It's exactly the same reasoning Google used in the linked example and which is used when building translations cache in Symfony: it's up to the developer to configure it correctly. |
@stof looking at this a bit more, it seems more components ship locale-related resources (for example It might make sense to ship this as a FrameworkBundle command which could then be updated to do this work across installed components and would also know which locales are required for the current app. |
@ro0NL I've updated this issue with an example, this should take into account your comments, but also provide the value. |
im still not convinced it's wise to mix&match "app enabled locales" and "iso available locales" generally you'd expect any iso locale to be available (and avoid the footgun scenario described in #51443 (comment)) BUT if one really prefers shrinking diskspace, then yes this likely helps maybe we can compress all data even smaller? 😅 |
From my experience the opposite is true: the vast majority of apps are using one locale and one locale only. Adding support for additional locales into your app is typically a sizeable process which includes stakeholders, designers, etc, it's not a "developer decision". Arbitrary locales are IMO a myth and very few apps need support for more than a few locales at a time.
That's a weird POV to me TBH, of course people care: googleapis/google-api-php-client-services#595 Huge deployments are harder to manage, run into more problems and are overall more annoying. If you can reduce the disk size of anything without it really impacting you in a significant way, you absolutely should, it just makes things easier. Symfony as a framework needs wide coverage and is correct to offer all the possible locales you can think of, of course. That doesn't mean your app should just have all those locales if you only use one or two. |
@ro0NL FYI I've done this manually and reduced my vendor/ folder by 25%. 👼 |
Thank you for this issue. |
Just a quick reminder to make a comment on this. If I don't hear anything I'll close this. |
Hey, I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen! |
Uh oh!
There was an error while loading. Please reload this page.
Description
In #50055 a new
compress
tool was added. It compresses the dataset and reduces the amount of disk space required, especially important for Docker based deployments.There exists an opportunity to provide this command as a Composer hook, like Google's API Services package does. This would make the experience for the users more streamlined.
Additional opportunity is to (opt-in) expand the command/hook to remove locales which are not in
framework.enabled_locales
, much like the translation cache is not built for them.Example
The text was updated successfully, but these errors were encountered: