Desktop: Improve Webpack resolve configuration

[spen]

The aim of these changes is to improve the resolve configuration of desktops webpack.config.js.
Though moduleDirectories works for us, it's not quite the correct place to place our application code - instead, these should configured as root modules.
There's a better explanation in this thread webpack/webpack#472, but here is the main take away:

resolve.root is an absolute path to a directory containing modules.
resolve.moduleDirectories is an relative path to a tree of directories containing modules.

As a happy side effect, these changes actually give us a decent (~6-11%) reduction in bundle processing time due to the way Webpack attempts to resolve paths using paths within modulesDirectories (check out this comment webpack/webpack#472 (comment) for some insight).
By running webpack builds with & without these changes a few times I got these timings:

Before	After	Speedup	%
29906ms	26372ms	3534 ms	11.8%
26095ms	23790ms	2305 ms	8.83%
25796ms	23594ms	2202 ms	8.54%
25676ms	23645ms	2031 ms	7.91%
25644ms	23990ms	1654 ms	6.45%

For anybody interested in running the build themselves (run from /desktop), make sure to include the NODE_PATH:

NODE_PATH=../server:../client webpack --config webpack.config.js

extensions: [ '', '.js', '.json', '.jsx' ],

Ideally we wouldn't need to add .json to the resolve.extensions array but there's an issue with resolving a json file in node-wpcom-oauth otherwise.
I have a PR open that should fix the reference (Automattic/node-wpcom-oauth#12) and allow us to remove .json from the list of extensions.

[dmsnell]

what a great find @spen!

do you think we could build with and without this PR applied and compare the output JavaScript? I wonder if they are character-for-character identical or not. are there any size differences?

[spen]

I remember the Webpack reports reading the same, but they do do some rounding - if I remember right the bundles were something like 5.72mb before and after, comparing with more fidelity would certainly be worthwhile.
I'll take a closer over the weekend :)

[spen]

Confirmed! Before === after :)

(I just renamed to bdesktop.js to avoid overwriting it on build)

Diffing in FileMerge also show no differences 👍

[dmsnell]

This is great @spen - a zero-diff means we should have high confidence here. Thanks for performing that check. I'm comfortable merging this but I would strongly recommend running it by @gwwar and @ockham and anyone else they recommend so that they can chime in and be aware of the change.

Thanks again!

[spen]

NP :) Thanks a bunch for taking a look @dmsnell!

[gwwar]

I love faster builds. Changes here seem fine to merge, especially since the target branch is another feature branch. Local dev and desktop seem to spin up great. @spen was there more info on setting the node path? NODE_PATH=../server:../client webpack --config webpack.config.js

Also summoning @blowery and @aduth if they had more insights.

[spen]

was there more info on setting the node path? NODE_PATH=../server:../client webpack --config webpack.config.js

Sure thing! :)

In wp-desktopthis short conversation is around setting up a similar config to NODE_PATH: Automattic/wp-desktop@edf7881#r105904602

In the target branch I've edited this to look up one level now that the relationship between Calypso and desktop have flipped around: https://github.com/Automattic/wp-calypso/pull/12430/files#diff-b1cd36ac3f156ef4dc61bff0f03cf429R98

[dmsnell]

@spen if the NODE_PATH isn't essential to Calypso or this PR and isn't related to the changes, let's - I think this is a simple and straightforward win here

[spen]

I'd say it's unrelated (at least directly).
I'm having some issues with either the NODE_PATH or the webpack config actually - but I'll make sure that's mentioned on the feature branch as I'm 99% sure that this change-set isn't effected by it :)

[dmsnell]

I'm having some issues with either the NODE_PATH or the webpack config actually - but I'll make sure that's mentioned on the feature branch

can you help explain that? basically I want to know if there's any reason we shouldn't go ahead and merge this in Calypso

[spen]

Ahh, this PR is based off the desktop merge feature branch and only effects the desktop/webpack.config.js - the root webpack resolver config for Calypso is already a-ok :)

[blowery]

LGTM

[spen]

Thanks gang <3