ahmadawais / create-guten-block

πŸ“¦ A zero-configuration #0CJS developer toolkit for building WordPress Gutenberg block plugins.
https://Awais.dev/cgb-post
MIT License
3.15k stars 326 forks source link

Change webpack compression levels? #76

Open Ipstenu opened 6 years ago

Ipstenu commented 6 years ago

Is it possible to change the compression level on webpack? I know it’s possible to tweak it by changing the level like this:

function gzipMaxCompression(buffer, options, callback) {
  return zlib['gzip'](buffer, {level: 9}, callback);
}

But I’m not sure where to tell the builder to dial it back a little. For smaller plugins, I think it would be acceptable to encourage readability a bit more.

ahmadawais commented 6 years ago

@Ipstenu there's no compression when you run npm start β€” so, if readability is what you're after you can use the code without compression there and don't run npm run build at all.

I suppose it's for learning purposes only coz I wouldn't recommend not compressing the JavaScript. It's already a huge factor in page load time and costs the most out of CSS/HTML/JS.

Peace! ✌️

Ipstenu commented 6 years ago

Oh sure, and for the most part, I would absolutely be for compression, but like I said, there are granular levels of compression from none to 9 (at least I think it's still 9...) and seeing as there actually is an expectation of some readability when JS is hosted on WordPress.org, it would be helpful to allow this to be tweaked by a config :) Default as it is now, which is what most people will want and need, and then less so for others who know what they're doing 😁

Ipstenu commented 6 years ago

Just as an example, this:

sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiMS5qcyIsInNvdXJjZXMiOlsid2VicGFjazovLy8uL3NyYy9ibG9ja3MuanM/N2I1YiJdLCJ[...]BQUFBO0FBQUE7QUFDQTtBQUNBO0FBQ0E7QUFDQTtBQUNBO0FBQ0E7QUFDQTtBQUNBO0FBQ0E7QUFDQTtBQUNBO0FBQ0E7QUFDQTsiLCJzb3VyY2VSb290IjoiIn0=\n//# sourceURL=webpack-internal:///1\n");

Is there a reason that HAS to be compressed to Base64? Some people will use code like that to hide things in their distributed code, so it's generally frowned upon when avoidable.

ahmadawais commented 6 years ago

Oh I see what you mean. Actually, the sourcemaps are being used in the production.

That is optional actually.

https://github.com/ahmadawais/create-guten-block/blob/master/packages/cgb-scripts/config/webpack.config.prod.js#L29

Anyone can define a an env based config and make

GENERATE_SOURCEMAP=false

and it should not add source maps in the production code. I haven't tested this feature yet coz I ran out of time I could spend on a free project. But feel free to test it and let me know.

βœ… If it works, I can accept a PR for README.md documenting this config feature. ❌ If it doesn't work, then I'm open to accepting PR's for it to make it work (πŸ€” I think dotenv file is not being read, if it's read then we can use this. Otherwise, normal env var should work)

Peace! ✌️

Ipstenu commented 6 years ago

I hear ya! It took a bit of pecking around, but it's actually in two places:

https://github.com/ahmadawais/create-guten-block/blob/master/packages/cgb-scripts/config/webpack.config.dev.js#L84

is the other one. I changed it to devtool: 'source-map', for a check and it threw the maps into a separate file.

So I think the actual issue is a little weirder.

  1. The only way to NOT super compress the files is to run in 'dev' mode (npm start)
  2. Dev Mode includes a SourceMap, which WebPack specifically tells you not to include in production, which means that you absolutely shouldn't use Dev Mode as is for production (besides it being a 'bad choice' though I think that's okay to leave up to the developers because ...)
  3. Making code readable is one of the important tenets of WordPress

So I would propose two changes (and I'm happy to make a pull request for these if they sound okay)

  1. Change GENERATE_SOURCEMAP to false for production
  2. Change devtool to hidden-source-map

This will bring the tool in compliance with best practices (i.e. no source map in production) and allow for people who want to build dev tools an easy way to both see the dev tool output (yay!) and omit it from Github/svn (.gitignore/svnignore for the win)

FWIW a quick benchmark for a plugin I'm working on, with Sourcemap false, the file sizes after G-Zip

With it true:

So yes, it's a little bigger (which is hilarious because the CSS files are 100% the same but whatever computer) but I think that would be an acceptable trade off given all things.

Note: This doesn't create a dev option (the dotenv didn't work, or I was doing it absolutely wrong) but it does bring the compression in alignment with best practices all around.

ahmadawais commented 6 years ago

πŸ€” Pretty sure that source-maps in production are not a bad practice. It's a matter of opinion. But I can see the reason why they shouldn't be on WordPress.org.

🎯 I think the .env vars are not being read and once they are being read by create-guten-block we'll be able to give users small configurable options. Very limited, coz it leads to a maintenance nightmare for me. It's a FOSS project after all.

πŸ‘‰ Which means, there needs to be a file that handles the reading of all types of .env files and then combines all them, remove the warnings, and stringifies them in a module.exports object.

That file included in the webpack config will make this source maps thingie configurable.

That's the basic idea. If you can work a PR for this β€” I'd be happy to merge it.

After the feature gets merged a dev would be able to create a .env file in the project root with GENERATE_SOURCEMAP=false as its contents to ignore the sourcemaps.

Peace! ✌️

ahmadawais commented 6 years ago

Also, the maps could be built in separate files but that's not the case here. Not sure why. If we can do that, those maps could be git ignorable.

P.S. Side-note, I know you come from a WordPress background and are writing webpack with capital W and P but the word webpack is all lowercase "even if it's the first word in the sentence β€” webpack's authors".

Cheers!

Ipstenu commented 6 years ago

I blame autocorrect for Webpack ;)

Pretty sure that source-maps in production are not a bad practice.

Actually it is per webpack's own docs: https://webpack.js.org/configuration/devtool/#production

You should configure your server to disallow access to the Source Map file for normal users!

and

You should not deploy the Source Map file to the webserver. Instead only use it for error report tooling.

Sadly the creation of making this read a .env file is outside of my current skill set. This is actually the fifth (yes 5) time I've ever made anything with javascript, and my first in react. I'll bumble around with it, but if anyone can sort it out sooner, please go for it!