Skip to content

[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

Closed
dkarlovi opened this issue Aug 21, 2023 · 21 comments
Labels

Comments

@dkarlovi
Copy link
Contributor

dkarlovi commented Aug 21, 2023

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

# compress locale only
bin/console maintenance:locale

# compress + delete any locale not in framework.enabled_locales
bin/console maintenance:locale --leave-enabled-only

# compress + delete any locale not in framework.enabled_locales, but don't delete fr, even if not in framework.enabled_locales
bin/console maintenance:locale --leave-enabled-only --leave fr
@dkarlovi dkarlovi changed the title Add a Composer hook to run intl/bin/compress + optionally remove languages not in framework.enabled_locales [intl] Add a Composer hook to run intl/bin/compress + optionally remove languages not in framework.enabled_locales Aug 21, 2023
@stof
Copy link
Member

stof commented Aug 21, 2023

The hook is easy, as Composer allows running any command as hook:

{
  "scripts": {
    "post-install-cmd": [
      "php ./vendor/symfony/intl/Resources/bin/compress"
    ]
 }
}

@stof
Copy link
Member

stof commented Aug 21, 2023

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.

@dkarlovi
Copy link
Contributor Author

@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.

@ro0NL
Copy link
Contributor

ro0NL commented Aug 21, 2023

note application enabled locales and available intl locales are separate lists, they dont correlate per se

@dkarlovi
Copy link
Contributor Author

@ro0NL how so, why would I want some locale in intl which is not enabled in the app?

@ro0NL
Copy link
Contributor

ro0NL commented Aug 21, 2023

because the intl data can be used by other means, regardless of the locales your app supports for its own i18n purposes

@dkarlovi
Copy link
Contributor Author

In what ways exactly?

@ro0NL
Copy link
Contributor

ro0NL commented Aug 21, 2023

Countries::getName($someCountry, $someLocale)

another issue is form integration, an app might use the LocaleType where any locale can be logically selected, even though the app its own i18n is available in less locales

@dkarlovi
Copy link
Contributor Author

In which case you don't opt into the cleaning, of course.

@ro0NL
Copy link
Contributor

ro0NL commented Aug 21, 2023

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)

@dkarlovi
Copy link
Contributor Author

Disk space is not little gain.

@ro0NL
Copy link
Contributor

ro0NL commented Aug 21, 2023

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)

@dkarlovi
Copy link
Contributor Author

as soon as any locale but an 'app enabled one'

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.

@dkarlovi
Copy link
Contributor Author

@stof looking at this a bit more, it seems more components ship locale-related resources (for example Validator).

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.

@dkarlovi dkarlovi changed the title [intl] Add a Composer hook to run intl/bin/compress + optionally remove languages not in framework.enabled_locales [FrameworkBundle] Add a Composer hook to compress + optionally remove locales not in framework.enabled_locales Aug 22, 2023
@dkarlovi
Copy link
Contributor Author

@ro0NL I've updated this issue with an example, this should take into account your comments, but also provide the value.

@ro0NL
Copy link
Contributor

ro0NL commented Aug 23, 2023

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? 😅

@dkarlovi
Copy link
Contributor Author

generally you'd expect any iso locale to be available

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.

if one really prefers shrinking diskspace

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.

@dkarlovi
Copy link
Contributor Author

@ro0NL FYI I've done this manually and reduced my vendor/ folder by 25%. 👼

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Just a quick reminder to make a comment on this. If I don't hear anything I'll close this.

@carsonbot
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants