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

Compress when building #1908

Closed
nikolay-g opened this issue Mar 30, 2017 · 11 comments
Closed

Compress when building #1908

nikolay-g opened this issue Mar 30, 2017 · 11 comments

Comments

@nikolay-g
Copy link

As a part of npm run build, can I also compress in addition to minifying?

@gaearon
Copy link
Contributor

gaearon commented Mar 30, 2017

We could, what is the benefit (vs letting the server do it)?

@tbillington
Copy link

@nikolay-g Just add the compression command of whatever tool you need to the end of the build command in package.json.

@gaearon
Copy link
Contributor

gaearon commented Mar 30, 2017

I don't mind doing it by default really. Just curious what the benefits are.

@nikolay-g
Copy link
Author

When serving content from an object storage (i.e. without NodeJS server), it makes sense to deploy the already compressed content.

It this is done at the webpack level, then npm run build will be platform agnostic - we won't have to rely on platform specific compression scripts.

@baptistemanson
Copy link

Hello everyone,
I have two questions related to compressing resources on the hard drive:

  • How can compressing on the object store send the HTTP Response with the right "Content-Encoding" header?
  • How to support requests with "Accept-Encoding": "none" headers? Or newer Chrome with "Brotli" compression?

If you use AWS, Azure or Bluemix, they will compress it for you. @nikolay-g you shouldn't have to worry to do it manually in my opinion.

@fabiosussetto
Copy link

fabiosussetto commented Apr 19, 2017

@baptistemanson there things you mentioned are handled by the webserver (such as nginx), whic would read the file, detect/guess the appropriate mimetype and then send the static file to the client.
Sometimes it can be useful to gzip as part of the build step because some CDNs (notably Amazon S3) cannot gzip files for you, so you need to upload them already compressed.

@gaearon giving that this is not something everyone would need (I imagine the biggest use case being a S3 deployment), is it necessary to support from within RCA? It can be easily done with a custom command/script that can be added in package.json (for example, calling $ gzip from the command line)

@baptistemanson
Copy link

Something like this should work for you, right?

"scripts": {
  "build":"react-scripts build",
  "ship-to-s3": "gzip -r build && aws s3 cp -r build/ ... " 
}

Cloudfront, the CDN based on S3 by Amazon compresses for you. By looking at our analytics, 5% of users do not accept gzip as a valid encoding right now in prod for our websites.

@Timer
Copy link
Contributor

Timer commented Apr 19, 2017

Hi guys!

I'm going to close this because it's probably not something we're going to support by default. Using @baptistemanson's suggestion is the way to go.

Let me know if there's a compelling reason to reconsider.

@kevinrobinson
Copy link

kevinrobinson commented Oct 24, 2017

This was surprising to me. Right now this guides people towards shooting themselves in the foot, especially with the deployment recommendations in the README and in Heroku's docs. There are two other issues about it (#3169 and #1117) with folks saying this is unexpected as well.

Worse, the output from the build task implies that the assets are gzip compressed when they aren't (added in #2648):

$ yarn run build
...
File sizes after gzip:

  209.55 KB  build/static/js/main.951a4711.js
  1.77 KB    build/static/css/main.bdd787db.css
...
$ ls -alt build/static/js/
-rw-r--r--  1 krobinson  staff   703530 Oct 24 08:40 main.951a4711.js
-rw-r--r--  1 krobinson  staff  4932846 Oct 24 08:40 main.951a4711.js.map

One option could be to update the docs to recommend gzipping, and updating the docs for each deployment option recommended in the README.

Another option could be to add in https://github.com/webpack-contrib/compression-webpack-plugin to https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/config/webpack.config.prod.js#L263 so that yarn run build produces both the plain and gzipped artifacts. The downside to adding this by default is that it adds time to the production build for folks who don't want gzipped assets. This seems like a good tradeoff - folks following the README and docs for create-react-app get gzipped assets in production by default, while increasing production build times for people who handle gzipping some other way. This also seems in line with the create-react-app philosophy to help teach and nudge folks towards learning best practices, and add sensible defaults that helps them avoid common mistakes.

Folks have mentioned that it's common for gzipping to be done on-the-fly by the webserver. FWIW, in my experience I haven't seen this in practice and compressing at build time has been more common. If folks adding recommended deployment options to the create-react-app docs missed this, then it's probably a place where a default could be helpful :)

@gaearon mentioned being open to pull requests in #1117 (comment), I'm happy to add in https://github.com/webpack-contrib/compression-webpack-plugin if folks are open to it.

@dbertella
Copy link

I think a postbuild script in the package json does the job quite well no need for anything else, maybe just add it in the documentation can be good for everybody?

@corysimmons
Copy link

@kevinrobinson

tumblr_oywdr5vrzm1w9vslno5_400

@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants