Use Webpack instead of Browserify for build process

[adamsilverstein]

#40834 kicked off a discussion of JavaScript modules in WordPress core.

In a recent ​JavaScript Core chat the decision was made to move in the direction of using Webpack (and ES6 imports) as our bundler of choice, and to work first on switching out browserfy in our current build chain.

Since we split the media files in #28510, the core build process has used Browserify to combine the media files. While browserify has served us well, Webpack is probably a better long term choice for the project, especially with the introduction of a new JavaScript framework that may require a build.

[adamsilverstein]

In 40894.diff​:

Change from browserify to webpack to build media.

• adds a new webpack.config.js
• type grunt webpack to run from grunt. Also runs as part of watch and in js:precommit

[adamsilverstein]

@kadamwhite can you take a look at my webpack setup and see if there is anything wrong or that we could do better - I want to keep it very simple at first, then we can iterate and improve - right now i am only trying to match wat browserify is doing for us.

thanks!

[adamsilverstein]

Appreciate testing here from anyone who can build and verify the built files look ok. i did some initial testing and everything worked as expected.

[aduth]

The Webpack 3.0 major release was published recently and we might ought to consider targeting it instead

​https://medium.com/webpack/webpack-3-official-release-15fd2dd8f07b

The grunt-browserify dependency should be removed from package.json

[adamsilverstein]

Thanks for the feedback and for pointing out webpack 3.0 @aduth, I will work on it!

[netweb]

Is there a reason the webpack dependencies are in dependencies and not devDependencies in package.json?

[netweb]

@adamsilverstein I've update the patch here with a couple of minor tweaks 40894.1.diff​ :

• Adds webpack.config.js to the grunt watch:config task
• Adds webpack.config.js to the grunt precommit task
• Moves the webpack dependencies from dependencies to devDependencies

I've not yet tested the output differences from switching to webpack, will test that in the coming days.

[adamsilverstein]

Replying to netweb:

@adamsilverstein I've update the patch here with a couple of minor tweaks 40894.1.diff​ :

thanks, i'll make my updates to your patch.

[adamsilverstein]

40894.2.diff​

• upgrade webpack to 3.0.0, add ModuleConcatenationPlugin to config
• remove grunt-browserify dependency
• slight code standards cleanup

[kadamwhite]

I'm reaching out to some Webpack contributors for additional review

[adamsilverstein]

Replying to kadamwhite:

I'm reaching out to some Webpack contributors for additional review

@kadamwhite - Any update here? I'd love to get this committed soon so we can get the new built files into trunk for testing.

[kadamwhite]

@adamsilverstein I ran this by Z Goddard for webpack review, and the only comment I got was that ModuleConcatenationPlugin is meant to only be used in a production setting -- all of the optimization plugins do a certain amount of analysis, so having them run in dev or during a --watch slows things down. If we split the config into a dev and prod config, we could also consider adding some of the other optimizations built in to Webpack down the road.

[adamsilverstein]

Replying to kadamwhite:

If we split the config into a dev and prod config, we could also consider adding some of the other optimizations built in to Webpack down the road.

Thanks for the review - great suggestion, I'll work on that!

[adamsilverstein]

In 40894.3.diff​:

• split config into two files - dev and production
• use dev config for watch
• remove webpack.optimize.ModuleConcatenationPlugin use from dev build

Tested out the built files in several browsers, ran tests etc. with good results. @kadamwhite - does this look good to you? Thinking commit this first, then a second commit updating the built files.

[netweb]

Patch 40894.4.diff​ is a refresh 40894.3.diff​ to apply cleanly against trunk (no changes) 👍🏼

[aduth]

A higher level question:

• Maybe not in the scope of what's intended here (simply replacing Browserify), but eventually what role would Webpack seek to serve? For example, the Grunt task for uglification is one which Webpack has built-in support (webpack.optimize.OptimizeJsPlugin). The devil's advocate question is: if it's just for basic bundling, are we gaining much over Browserify (+Babelify)?

A couple thoughts on the configuration:

• At some point if we start to observe significant overlap between development and production configurations, we might want a more scalable approach to sharing common properties (like a common configuration). Conversely, if this is not something we want to aim for early on, we may not even need the file split right now. Gutenberg, for example, has a single webpack.config.js which switches on the process.env.NODE_ENV to add/modify configuration values. This could help with avoiding redundancy on assigning mediaConfig.
• Should watch: true be assigned into the configuration from Gruntfile.js, or does that make more sense within the configuration itself?
• We specify stats dependant on the process.env.NODE_ENV but it doesn't appear we set this NODE_ENV anywhere, so it seems unnecessary. And even if it did work as expected, would we want nothing to be output for production builds?

A few minor styling quibbles (in Webpack config files):

• Should we bother with space aligning variable and object property assignment? Especially with comma-delimited variable declarations, it makes the incorrect assumption that a tab occupies four spaces (in Trac patch, it's obvious with the misalignment since it previews tabs at 8 spaces). Also that we have two spaces following the longest variable name in the respective Webpack configurations.
• Should be spaces between array square brackets (mediaBuilds) and in require calls, and not after function keyword
• module.exports object should be concluded with a semi-colon.

[aduth]

I guess the obvious response to my own question about scope of Webpack is that if Gutenberg is to be merged with its proposed build tooling (Webpack-based), taking steps in core to accommodate this would ease such a transition. A more interesting question to eventually consider then might be: What does this configuration look like when it scales to accommodate building both media and Gutenberg?

[adamsilverstein]

@aduth

Thanks for the review, feedback and questions.

I don't think we need to add minification here, we already handle that in our main build process which we run to package releases. For development, the unminified version is preferable and this approach fits nicely with our existing build tool chain.

One additional use I imagine for webpack is importing/building our ​npm packages in the wp namespace for inclusion in core.

Regarding splitting the configuration files, I had originally tried a single file and split it up based on this comment and thinking it may help optimize the production/dev builds down the road.

I will try to clean up the configs, removing stats and maybe moving watch:true as well as fixing your other noted format items.

What does this configuration look like when it scales to accommodate building both media and Gutenberg?

Thats a great question we need to figure out the answer to - hopefully we can port over much of what exists in the project. Having webpack in our core toolset will be a prerequisite.

[westonruter]

A follow-up ticket to build on the work being done here: #41870 (Code Editor: Add grunt task for building new CodeMirror bundles from external dependencies)

[adamsilverstein]

In 40894.5.diff​:

Address feedback from @aduth -

• whitespace fixes
• remove unused options->stats
• add missing trailing semicolons

[adamsilverstein]

40894.6.diff​ includes the rebuilt media files. The generated media-views file is slightly larger, probably due to webpack's inline comments. Once run thru minification, the built file is actually about 5% smaller than the file browserify built:

I did basic testing of media with these webpack built files in ie down to version 9.

[schlessera]

@adamsilverstein The latest patch seems to remove the Hello Dolly plugin. I suppose that was not intended?

[adamsilverstein]

@schlessera Thanks for testing... No, that was not intentional, let me re-upload that patch.

[adamsilverstein]

@schlessera can you please try testing again with 40894.7.diff​?

[netweb]

Webpack is pretty new to me and I've not yet grok'd all the things.... That said.... Please bare with me why I explain my testing timeline....

When I tested 40894.3.diff​ and refreshed this to become 40894.4.diff​ I tested this by converting the /build directory to a Git folder ( git init && git add . && git commit), I then renamed the folder to/build-git and ran grunt release from the root of the repo to recreate the /build folder, I then copied the contents of /build to /build-git to compare the differences via git status, in my testing of this there were no changes so I refreshed the patch and uploaded 40894.4.diff​ and added no additional comment apart for a "thumbs up" to indicate the patch as it stood worked for me as I saw no noticeable changes before and after the patch in the output /build folder.

Moving along, 40894.5.diff​ at a cursory glance is quite similar to 40894.4.diff​ and 40894.3.diff​ ...

And then to 40894.6.diff​, this contains significant changes to the src/wp-includes/js/media-audiovideo.js, src/wp-includes/js/media-grid.js, src/wp-includes/js/media-models.js, and src/wp-includes/js/media-views.js files, I originally expected these changes in the 40894.3.diff​ patch, this is why I performed the testing of the 40894.3.diff​ patch the way I did above and I thought the reason for why those changes were not in 40894.3.diff​ was because I wasn't grok'ing webpack correctly and with no difference between before and after applying the patch I thought the changes were good enough, maybe perfect enough that the output to the /build folder had no changes whatsoever and I moved along with nothing worth noting here in the comments.

So, all in all I hadn't grok'd webpack very well and the changes I expected are in fact now in 40894.6.diff​ and 40894.7.diff​, so for whatever reason this is I'll read and re-read and test some more so I do grok what these changes actually are, for now though as I'm basically on holidays with family for the next 6 days I'm unable to test the 40894.7.diff​ patch.

If others are happy with the state of 40894.7.diff​ then commit away, I'll catch up when I get back from holidays but I wanted to document the above for my own reference at least... Maybe next week a light bulb moment will occur and I'll see what I clearly missed previously but don't let me hold this back :)

[adamsilverstein]

@netweb Ha! Thanks for the detailed testing log :) 40894.7.diff​ is what you should be testing now - it includes the change to the build script as well as the changes to the built media files resulting from the build changes. In the earlier patches, I had left off the built files to make the patch easier to read.

I'm inclined to commit this to get it out to a wider testing audience in beta. If we discover a compatibility issue, we can always revert.

[netweb]

Replying to adamsilverstein:

In the earlier patches, I had left off the built files to make the patch easier to read.

Well that's what threw me, these should always be included IMHO regardless of how well things read or not.

I'm inclined to commit this to get it out to a wider testing audience in beta. If we discover a compatibility issue, we can always revert.

As noted I'm travelling for the next week, it would be nice to get a couple of sets of eyes on this before commit IMHO

[adamsilverstein]

40894.8.diff​ refresh against trunk.

[adamsilverstein]

remove unrelated changes in gruntfile after merge conflict resolution

[kadamwhite]

Notes from slack review with @adamsilverstein :

• webpack.config.dev.js and webpack.config.prod.js are more standard than webpack-dev.config.js, I'd err on the side of convention here (as established in projects like create-react-app and comments in the webpack docs) for the added specificity
• We should be adding the most up-to-date versions of webpack and webpack-dev-server, they've both evolved a point or patch release or two since this patch was created

Other than that, looks good, let's do it!

[adamsilverstein]

In 40894.10.diff​:

• Rename config files to webpack.config.dev.js and webpack.config.prod.js.
• Update to the latest version of webpack and webpack-dev-server.

[adamsilverstein]

In 41752:

Build: Webpack instead of Browserify as JavaScript bundler.

Use Webpack as our bundler of choice, enabling improved JavaScript build chain capabilities.

Props aduth, netweb, kadamwhite, schlessera.
Fixes #40894.