Merging wp-desktop with wp-calypso: Folder structure & webpack resolvers

[spen]

I've been tinkering with my first pass at bringing the desktop build process in to Calypso itself.
There are many questions to be answered: how will versioning work? How often should desktop be built how do we build desktop with a specific snapshot of Calypso?; but for now, I just want to tackle some initial hurdles that we'll hit regardless of those logistics.

To me, the most obvious of these is where will the desktop build files live within the Calypso project?
My first thought is to just create a directory at the root level called /desktop but I did want to throw this out there and see if anybody can give a good reason why now - maybe it's not descriptive enough, maybe it should be placed in one of the existing directories?

With this move comes a flip in relationship between the desktop code and Calypso itself.
In wp-desktop Calypso is a submodule (wp-desktop -> wp-calypso). After the merge wp-desktop will instead be a submodule of Calypso.
One of the immediate impacts of this is that the webpack config in wp-desktop no longer resolves correctly.

Currently, the wp-desktop config makes use of resolve.modulesDirectories like so:

modulesDirectories: [
	'node_modules',
	path.join( __dirname, 'calypso', 'server' ),
	path.join( __dirname, 'calypso', 'client' ),
	'desktop'
]

My concern here is that here we have 4 different directories resolving at a top level that could potentially conflict.
An interesting case is that calypso/server and calypso/client each have a ./boot directory.
wp-desktop/desktop/server/server refers to a boot file like so:

var boot = require( 'boot' );

I may be mistaken but I think it's by luck alone that this resolves as the calypso/server file and not that of calypso/client.

I think there may be value in being a little more explicit with imports/requires when referencing any 'Calypso' code from within the desktop, possibly setting up the webpack config file to enable something like the following instead:

var boot = require( 'calypso/server/boot' );

This doesn't solve the issue of having multiple directories share the same root-level referencing though but I'll continue to think of a better solution.

For starters though, I'm hoping to at least pull out all but node_modules from modulesDirectories and instead set the other directories to either roots or aliass - I plan to update a PR in wp-desktop with those changes once tested and working ( Automattic/wp-desktop#272 ).

I'd love to hear your thoughts on any of the above points :)

[spen]

Regarding the various options of webpacks resolve config here's A discussion worth noting, (particularly the linked comment)

[blowery]

I may be mistaken but I think it's by luck alone that this resolves as the calypso/server file and not that of calypso/client.

I don't think it's luck, the moduleDirectories entries are scanned in order. They work similarly to how NODE_PATH works in plain old node modules.

[spen]

Thanks @blowery, I think this is sort of what I was alluding to, but without knowing the details.
I meant luck as in, lucky that it's the first matching path found - for instance, requiring boot from, lets say, the desktop directory that I'm building does match with server/boot but this still seems loose, since we never specified anywhere that this is the correct boot (vs client/boot). If we had instead wanted to refer to client/boot from this place we would be out of luck (well, we would have to use a different path).

I think I've just been looking at this for a bit too long without any progress or resolution so I may try another approach! :)

[spen]

I do have one related idea that I'm keen to get thoughts on...
Treating /client /server and soon /desktop as roots/module-directories - it might be good practice to set up aliases so that any time a file in /server wants to require a file in /client it must do so with client/subdir/file - even just to avoid indirection.
The same would of course apply for any other crossing of these directories.

I think this sort of change could help me with my port as currently I'm trying to resolve this error:

uncaughtException (fatal) { Error: Cannot find module "../../client/sections"
    at webpackMissingModule (/Users/Spen/Projects/wp-calypso/wp-desktop/build/desktop.js:177379:82)

As a result of copying Calypso files in to a build directory.

[nb]

How often do we import code cross client/server? Is this really a concern?

Do you think there are any trade-offs of having a root /desktop directory next to /client and /server?

[spen]

How often do we import code cross client/server?

My concern is actually more about referencing /client or /server files from /desktop - I just feel that being more explicit in any cross-over file references might help avoid indirection.

I spent a while trying to work out which boot was being referenced and before I thought to check the webpack.config I even started thinking that it might be a core node module - partially because it was living next to require( 'http' ). Some details on that are here Automattic/wp-desktop#272.
My thinking was that if this was more specific a developer (such as earlier myself) could instantly see which file this was in reference to.
There are also 3 different directories that are currently resolvable as require( 'boot' ). I mention above that it's just by 'luck' that the right one is resolved in the desktop build - 'luck' isn't quite what I meant... I don't know much about NODE_PATH or webpacks resolve order ( @blowery mentions this above ) but if for instance we wanted to reference client/boot from desktop we would instead get server/boot.

Also, I think this is a similar (or the same?) pain point that @ockham was experiencing with certain paths being completely fine withing Calypso, but failing in wp-desktops build.

[ockham]

Do you think there are any trade-offs of having a root /desktop directory next to /client and /server?

FWIW, this would be my preferred solution.

Furthermore, I'd use Calypso's current webpack configs etc as the baseline and consider the ones in wp-desktop more as layers of duct-tape wrapped around (possibly dating back to a time when it wasn't possible to do differently), and possibly obsolete.

Also, I think this is a similar (or the same?) pain point that @ockham was experiencing with certain paths being completely fine withing Calypso, but failing in wp-desktops build.

Mine was the inverse situation -- I was importing client/something from withing a server/ file, because, you know, unambiguity 😉 Worked with Calypso, failed miserably with wp-desktop.

So my strategy would be to work out what is actual business logic inside wp-desktop (electron shell, menus, stuff like that), try and move that into a /desktop folder in Calypso, and try to make it work from there.

[spen]

I was importing client/something from within a server/ file, because, you know, unambiguity

This is exactly what I'd like to see happen!
I'm having yet another round of issues with the resolvers while trying a more 'from-the-ground-up' approach...

try and move that into a /desktop folder in Calypso, and try to make it work from there.

Good point - It's definitely not going to be a simple case of lifting-and-shifting - far from it I think :(

[spen]

It's worth mentioning here that I've opened a PR that relates to this issue here: #12537.
It also turned out that most of the issues that I was seeing early on were solved by setting NODE_PATH=../client:../server and updating some of the relative-path imports within /desktop to point to the right targets.

I still think that there might be value in adding aliases to allow for less ambiguous paths (boot => client/boot for example) but for now everything resolves as expected!

Closing this for now, I'll open more targeted issues relating to the above in the future if necessary :)