Skip to content

Conversation

Mr0grog
Copy link
Member

@Mr0grog Mr0grog commented Jan 7, 2018

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:

  • Live rebuilding via dev middleware (so you no longer need to run both the server and a separate build process as before)
  • PostCSS Autoprefixer
  • JS minification/uglification/gzip

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 the env preset applies and what prefixes the autoprefixer adds to CSS has to be duplicated across package.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 the env preset that will be included with Babel v7 has support for pulling the browserslist config from package.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

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.
@Mr0grog Mr0grog changed the title WIP: Build with Webpack; Remove Gulp & Browserify Build with Webpack; Remove Gulp & Browserify Jan 7, 2018
@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 7, 2018

OK, I think this is good to go now.

Copy link
Collaborator

@lightandluck lightandluck left a 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?

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 9, 2018

I can't seem to install zopfli-webpack-plugin and seems like others have had the same problem and error message I was getting.

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?

might not be maintained anymore. Last commit was Nov. 2016.

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 :\

update the deploy script and README to use the new build commands?

Oops, yes, I should have done that.

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 9, 2018

Updated README and deploy script.

@lightandluck
Copy link
Collaborator

I tried to install using npm, just to see if it would work and got an error:

gyp ERR! stack Error: Python executable "/Users/FookNasty/miniconda3/bin/python" is v3.6.3, which is not supported by gyp.
gyp ERR! stack You can pass the --python switch to point to Python >= v2.5.0 & < 3.0.0.

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???

error /Users/FookNasty/Desktop/EDGI/web-monitoring-ui-lightandluck/node_modules/node-zopfli: Command failed.
Exit code: 1
Command: node-pre-gyp install --fallback-to-build
Arguments:
Directory: /Users/FookNasty/Desktop/EDGI/web-monitoring-ui-lightandluck/node_modules/node-zopfli
Output:
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using [email protected]
node-pre-gyp info using [email protected] | darwin | x64
node-pre-gyp info check checked for "/Users/FookNasty/Desktop/EDGI/web-monitoring-ui-lightandluck/node_modules/node-zopfli/lib/binding/node-v57-darwin-x64/zopfli.node" (not found)
node-pre-gyp http GET https://node-zopfli.s3.amazonaws.com/Release/zopfli-v2.0.2-node-v57-darwin-x64.tar.gz
node-pre-gyp http 403 https://node-zopfli.s3.amazonaws.com/Release/zopfli-v2.0.2-node-v57-darwin-x64.tar.gz
node-pre-gyp ERR! Tried to download(403): https://node-zopfli.s3.amazonaws.com/Release/zopfli-v2.0.2-node-v57-darwin-x64.tar.gz
node-pre-gyp ERR! Pre-built binaries not found for [email protected] and [email protected] (node-v57 ABI, unknown) (falling back to source compile with node-gyp)
node-pre-gyp http 403 status code downloading tarball https://node-zopfli.s3.amazonaws.com/Release/zopfli-v2.0.2-node-v57-darwin-x64.tar.gz
node-pre-gyp ERR! build error
node-pre-gyp ERR! stack Error: Failed to execute 'node-gyp clean' (Error: spawn node-gyp ENOENT)
node-pre-gyp ERR! stack     at ChildProcess.<anonymous> (/Users/FookNasty/Desktop/EDGI/web-monitoring-ui-lightandluck/node_modules/node-pre-gyp/lib/util/compile.js:77:29)
node-pre-gyp ERR! stack     at emitOne (events.js:116:13)
node-pre-gyp ERR! stack     at ChildProcess.emit (events.js:211:7)
node-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:196:12)
node-pre-gyp ERR! stack     at onErrorNT (internal/child_process.js:372:16)
node-pre-gyp ERR! stack     at _combinedTickCallback (internal/process/next_tick.js:138:11)
node-pre-gyp ERR! stack     at process._tickCallback (internal/process/next_tick.js:180:9)
node-pre-gyp ERR! System Darwin 15.6.0
node-pre-gyp ERR! command "/usr/local/lib/node_modules/node/bin/node" "/Users/FookNasty/Desktop/EDGI/web-monitoring-ui-lightandluck/node_modules/node-zopfli/node_modules/.bin/node-pre-gyp" "install" "--fallback-to-build"
node-pre-gyp ERR! cwd /Users/FookNasty/Desktop/EDGI/web-monitoring-ui-lightandluck/node_modules/node-zopfli
node-pre-gyp ERR! node -v v8.9.4

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 9, 2018

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 :\

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 9, 2018

Anyway, if that still doesn’t work for you, I’ll just switch it to Gzip and cry softly to myself about lost bytes ;)

@lightandluck
Copy link
Collaborator

lightandluck commented Jan 9, 2018

Haha, just got it working. Creating a new environment worked. The command, for me at least, to switch environments was source activate node-compile. I was then able to install with npm from there, but was still getting errors from yarn.

I found this workaround yarn global add node-gyp and that got the package to build with yarn. Still a little confused because I found this PR which says that yarn should add node-gyp itself, but ¯_(ツ)_/¯. I have the latest version of yarn 1.3.2.

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.

@lightandluck
Copy link
Collaborator

I'll have to review the actual build process tomorrow. 😴

@lightandluck
Copy link
Collaborator

Everything looks great except when I pushed to staging, I was getting this warning:

Warning: It looks like you're using a minified copy of the development build of React. When deploying React apps to production, make sure to use the production build which skips development warnings and is faster. See https://fb.me/react-minification for more details.

Followed the links and eventually got to this snippet in their webpack.prod.js. You'll understand better than I how to incorporate it into the config, but we should fix that warning.

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 10, 2018

D'oh, I had a commit I forgot to push last night that does that. (Now pushed)

@lightandluck
Copy link
Collaborator

Looks great! Thanks for doing this. I learned a ton reading through it.

@Mr0grog Mr0grog merged commit 23524b7 into master Jan 12, 2018
@Mr0grog Mr0grog deleted the 88-dont-take-such-big-gulps-and-just-webpack-it-in branch January 12, 2018 07:15
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.

Consider switching from gulp to webpack

2 participants