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

Update url-loader to 1.0.1 and adapt webpack build config #3779

Merged
merged 1 commit into from
Apr 18, 2018

Conversation

llienher
Copy link
Member

Fixes the two issues with webpack build we have on EPFL project.

Dependency update from url-loader 1.0.0 to 1.0.1 fixes an issue with urls being processed as [Object object] instead of the wanted base64 inlined data strings.

Use of file-loader instead of url-loader allow the production css file to have static relative urls instead of base64 inlined strings for the png, svg, eot, woff and woff2 files format. This change allowed in EPFL to reduce css from 2 Megabytes to 250 Kilobytes.

I'm not sure if we should do it for .cur files, probably it is not a huge gain as there is only 2 files and probably not many added in projects...

@llienher llienher added this to the 2.3 milestone Apr 18, 2018
@llienher llienher self-assigned this Apr 18, 2018
@@ -131,7 +131,7 @@ const htmlRule = {
const iconRule = {
test: /\.(png|svg)$/,
use: {
loader: 'url-loader'
loader: 'file-loader'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should add a limit instance of every-times use the file-loader!

@sbrunner
Copy link
Member

The css size isn't a criteria, we should take care on the full download size and the number of downloaded files...

@llienher
Copy link
Member Author

With file-loader:

-rw-rw-r--. 1 llienher sigdev 247K Apr 18 14:06 epfl_authgeoportail/static-ngeo/build/pro.css
-rw-rw-r--. 1 llienher sigdev 40K Apr 18 14:06 epfl_authgeoportail/static-ngeo/build/pro.css.gz

With url-loader:

-rw-rw-r--. 1 llienher sigdev 2.0M Apr 18 14:31 epfl_authgeoportail/static-ngeo/build/pro.css
-rw-rw-r--. 1 llienher sigdev 895K Apr 18 14:31 epfl_authgeoportail/static-ngeo/build/pro.css.gz

More generic, global loading results, use both same link and browser cache disabled.
With file-loader:
186 requests for 4.8MB, a bit quicker (between 3.5 and 4 sec).

With url-loader:
186 requests for 5.4MB, a bit slower (usually above 4 sec).

@sbrunner what is the best then ? Add the limit and use file-loader, or stay with url-loader and just keep the dependency update ? Considering EPFL is heavily customized css-wise, but quite fast to load on PC, while we don't use a lot the mobile interface, which is more sensitive to the loading data transfer and speed in general so we don't the global use-case preview with this project. :-)

@gberaudo
Copy link
Member

A CSS of 1MB gzipped is the sign something is going wrong:

  • unnecessary stuff loaded at startup instead of when it is actually used;
  • duplication (same file as data-uri can not be shared);
  • suboptimal caching (some resources never change, even between new versions, like some fonts).

If I look at current EPFL production, I see the CSS file is around 41KB gzipped. As a matter of fact, it contains almost no data:uri.

I am in favor of not changing the current default behaviour, and so to merge this PR.

It would be interesting to let applications configure the inlining behaviour; it can come in a future PR.

@llienher llienher merged commit 3ded8ed into master Apr 18, 2018
@llienher llienher deleted the fix-webpack-build branch April 18, 2018 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants