Skip to content
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

[Webpack/Moment] Include all moment locales #592

Merged
merged 3 commits into from Jun 19, 2017

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jun 15, 2017

This resolves #591 if someone else can confirm the same error.

The admin.js file size in production increases to 227k (from 178k)

The locale list comes from config.yml:

parameters:
    # This parameter defines the codes of the locales (languages) enabled in the application
    app_locales: en|fr|de|es|cs|nl|ru|uk|ro|pt_BR|pl|it|ja|id|ca|sl|hr|zh_CN

@alOneh
Copy link
Contributor

alOneh commented Jun 16, 2017

I was thinking about this solution, but if we had a new locale we have to update the webpack config...

@yceruto
Copy link
Member Author

yceruto commented Jun 16, 2017

Yep, we have few options, I would say, let's forget about the size of the admin.js file and thus avoid updating both lists, or generate that part inside webpack.config.js from cache: warmup, or add a comment over app_locales to not forget this?

@javiereguiluz
Copy link
Member

@yceruto yes, forgetting about the admin file size is one option ... the other one would be to remove moment.js entirely (I don't know what it does ... but it takes almost 400 KB, so it must do wonders).

@yceruto
Copy link
Member Author

yceruto commented Jun 16, 2017

yes, forgetting about the admin file size is one option

👍

[...] the other one would be to remove moment.js entirely

nop, it's a minimal requirements (http://eonasdan.github.io/bootstrap-datetimepicker/Installing/)

@yceruto yceruto changed the title [Webpack/Moment] Include used locales only [Webpack/Moment] Include all moment locales Jun 16, 2017
@javiereguiluz
Copy link
Member

Thank you @yceruto.

@javiereguiluz javiereguiluz merged commit 1818ab4 into symfony:master Jun 19, 2017
javiereguiluz added a commit that referenced this pull request Jun 19, 2017
…ereguiluz)

This PR was merged into the master branch.

Discussion
----------

[Webpack/Moment] Include all moment locales

This resolves #591 if someone else can confirm the same error.

The `admin.js` file size in production increases to `227k` (from `178k`)

The locale list comes from `config.yml`:
```yaml
parameters:
    # This parameter defines the codes of the locales (languages) enabled in the application
    app_locales: en|fr|de|es|cs|nl|ru|uk|ro|pt_BR|pl|it|ja|id|ca|sl|hr|zh_CN
```

Commits
-------

1818ab4 Removed an unneeded variable
774a4b8 [Webpack/Moment] Include all moment locales
0726e00 [Webpack/Moment] Include used locales only
@yceruto yceruto deleted the locale branch June 19, 2017 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: locale() locale es is not loaded from moment locales!
3 participants