-
-
Notifications
You must be signed in to change notification settings - Fork 40
Build with Webpack; Remove Gulp & Browserify #175
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
Build with Webpack; Remove Gulp & Browserify #175
Conversation
Build with `yarn run build` or `yarn run build-production` (for a final production build). Does not yet have live building (going to integrate the dev middleware next), but for now you can just run `webpack --watch` on the command line like you used to run `gulp img css browserify watch`.
This only runs in dev mode, which is the default. Set `NODE_ENV` to anything except `development` to disable. This also gets us back to feature parity (but implemented slightly nicer) with the previous Gulp/Browserify implementation, so: fixes #88.
This probably needs feedback on whether the browser configuration is "right".
This uses browserslist to apply specifically the transforms needed for the selected browsers (like autoprefixer does), rather than applying all the transforms for given ECMAScript spec versions. The spec version presets are being deprecated with the upcoming Babel v7 release: https://babeljs.io/blog/2017/12/27/nearing-the-7.0-release#deprecated-yearly-presets-eg-babel-preset-es20xx Note we have duplicated environment configurations between `package.json` and `.babelrc`. There's no reasonable way of avoiding this for now -- Jest compiles files directly with Babel instead of Webpack (compiling with Webpack is possible but involves a *way* more complicated setup; we don't need those features for our Jest tests yet), so it needs the `.babelrc` file around so that Babel can be configured independently of our Webpack file. The version of the `babel-preset-env` package that will be released with Babel v7 can read the `package.json` file to get this data (like autoprefixer already does), so we can remove this once that is released and we upgrade.
OK, I think this is good to go now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't seem to install zopfli-webpack-plugin
and seems like others have had the same problem and error message I was getting. Took a look and zopfli-webpack-plugin
has a dependency on node-zopfli
which might not be maintained anymore. Last commit was Nov. 2016. I don't know what alternative to go with.
In the meantime, can we also update the deploy script and README to use the new build commands?
Hmmm, it’s true that there are no pre-built binaries for most recent combinations of Node + operating systems + CPUs, but it should just go ahead and start building from source when it can’t find the binary. What’s the exact error message you’re getting after the “Pre-built binaries not found” one?
I’m not too worried about that as a problem for a module like this. Zopfli itself hasn’t had a new release in a couple years, but then neither has GZip or plenty of other reliable compression algorithms. We can switch to plain old GZip if people would feel better about that, though. It’s just not nearly as high a compression ratio :\
Oops, yes, I should have done that. |
Also update app to return correct content-type for gzipped files
Updated README and deploy script. |
I tried to install using npm, just to see if it would work and got an error:
Error I'm getting when using yarn to install below. It doesn't mention anything about python, so not sure if that might be related???
|
Ah, it requires Python 2 to build (built-in on OS X), but it looks like you are using Conda to have Python 3, which will override your system Python. You need to set up a Conda environment with Python 2 and then install while you are in that environment. So you might: $ conda create --name node-compile python=2.7
$ conda activate node-compile
$ yarn install # or npm install Not sure why the yarn version is not getting that far, though. It gave me the same, much clearer Python error you got with NPM when I first ran it :\ |
Anyway, if that still doesn’t work for you, I’ll just switch it to Gzip and cry softly to myself about lost bytes ;) |
Haha, just got it working. Creating a new environment worked. The command, for me at least, to switch environments was I found this workaround I think we should add this to the README in case anybody else has the same trouble. It sounds like you also had to build from source, but didn't run into the problems I did. |
I'll have to review the actual build process tomorrow. 😴 |
Everything looks great except when I pushed to staging, I was getting this warning:
Followed the links and eventually got to this snippet in their |
D'oh, I had a commit I forgot to push last night that does that. (Now pushed) |
Looks great! Thanks for doing this. I learned a ton reading through it. |
Quick implementation of Webpack that matches our previous build process with no extra frills (yet). I don’t think this branch should add too much new functionality, but we should implement a few more simple things before merging:
I wound up also adding Babel’s
env
preset here (instead of the ones for specific ECMAScript versions) because it turns out the presets for each version of ECMAScript are being deprecated soon. :(Sadly, the
browserslist
configuration that manages both what Babel transforms theenv
preset applies and what prefixes the autoprefixer adds to CSS has to be duplicated acrosspackage.json
and.babelrc
. Jest uses Babel without using Webpack (it can use Webpack but configuring that is a whole other ball of wax), so the.babelrc
file is required to keep the Babel configuration separate from Webpack. The version of theenv
preset that will be included with Babel v7 has support for pulling thebrowserslist
config frompackage.json
(and other places, too), but the current shipping version does not. So we can remove this duplication as soon as Babel v7 is out and we’ve upgraded.Also added asset compression with Zopfli (a better, but slower GZip). If we want to be extra fancy, we could add Brotli, too (much better compression, but not gzip-compatible).
It would be great to add support for hashes in filenames for production (so we automatically bust caches), but doing that requires a wee bit more fancy footwork.
Fixes #88