FullstackAcademy / boilermaker

Code scaffold for projects
https://www.youtube.com/watch?v=7bLSuTHH4Ag&list=PLx0iOsdUOUmn7D5XL4mRUftn8hvAJGs8H
MIT License
215 stars 706 forks source link

dev mode webpack #157

Open collin opened 5 years ago

collin commented 5 years ago

Problem

Our current start script runs like this:

npm run build-client-watch & npm run start-server

Webpack (build-client-watch) runs as a background job (that's what the single ampersand does). This creates a couple problems:

1: The background job can turn into a zombie process causing confusion while debugging 2: It is possible to reload a browser tab before the webpack process finishes. express will happily serve the stale bundle.js

Proposal

Use webpack-dev-middleware. Webpack ships a middleware that can be added with app.use.

Webpack dev middleware bundles the javascirpt just-in-time when it is requested. It's impossible(probably) to get a stale bundle. And there will be only one process for boilermaker projects.

Problems created

dev/deployed configuration

This will create a little more disconnect between the dev and deployed environments. It makes sense to build a bundle.js on disk for deployed environments, so we'll need to write our webpack configuration such that it can be used both by the middleware and by webpack command line. (this is not difficult)

inconsistent with other projects

We follow this pattern in some other projects, it would be best to find them and update them to follow this pattern as well. (senior enrichment, some later jr phase projects)

glebec commented 5 years ago

I'm not totally against this, though I'd ideally maybe like to preserve some way of optionally running the build and server as separate processes so I can teach about running them in separate terminal panes.

Also ideally, I would like to ensure that editing client files re-runs the build, but editing server files only restarts the server (and doesn't trigger a cold rebuild) – knowing how sophisticated the Webpack devs are in general, I would hope this is possible, but I haven't looked into it. EDIT: this isn't a must-have though.

These points are admittedly more geared towards having a flexible and efficient dev environment than a simple environment. I might however argue that conflating the build with the server, as in webpack-dev-middleware, is conceptually muddying for students even if it leads to fewer headaches during development.

It seems to me we need to decide what our priorities are vis-à-vis:

glebec commented 5 years ago

PS Re: stale bundle issues, there are a couple of approaches to consider:

collin commented 5 years ago

I think my strongest objection is our use of & to run the job as background. Students seem to have a knack for closing their terminals in ways that leave behind zombie processes.

glebec commented 5 years ago

I hear that. I actually try not to use that myself, because I don't like mixing the output of two separate processes in one terminal window. I would almost say we can just remove the combined script and instruct them to run separate build and server processes (at a minimum). Students should graduate understanding that sometimes they have to run multiple scripts in parallel…

…of course, if this happened I know we would get help tickets every single cohort with students forgetting to run the build / relying on a stale build. But maybe we shouldn't be optimizing for reduction of help tickets.

benwilhelm commented 5 years ago

I'm with Gabriel on this one. I think as an educational tool, it makes the most sense to just call them two separate processes and have the students run them in separate terminals. We could tell them that there are more sophisticated tools out there if they want to explore them, but I think we should be optimizing for understanding over simplicity of execution.

glebec commented 5 years ago

Oh and also there's the concurrently package, which color-codes the concurrent tasks by namespace (and does a whole bunch more besides). Might be worth considering. I think we used to have this in https://github.com/FullstackAcademy/bones, but eventually it got a bit overwrought and @queerviolet switched to using a dev.js script. That is adding a lot of complexity to a student platform however.

EDIT: found where the old dev npm script using concurrently was switched to running the dev.js file instead, see here.

-"dev": "cross-env NODE_ENV=development concurrently --kill-others --prefix \"[{name}]\" --names \"BUILD,SERVE\" -c \"bgBlue.bold.black,bgGreen.bold.black\" \"npm run build-dev\" \"npm run start-dev\"",
+"dev": "node dev"

Since then, it looks like the concurrently usage docs have been updated for some fancy features such as wildcards & npm run -> npm: prefixes, possibly making concurrently more usable.

queerviolet commented 5 years ago

Popping into this discussion to offer a comment of questionable value: on all my current projects, I've found that just using Parcel is totally the way to go for building a frontend. It doesn't have a quick way to connect to an express server, but it would be straightforward to write a parcel plugin that:

  1. Looks for ${bundler.options.rootDir}/server.js
  2. Requires it
  3. Calls it with bundler.server

(Parcel plugins are just functions which are called with the bundler.)

Bonus points if it also provides a command that will do this, only without the bundler and with an express server serving on $PORT and statically serving dist (which parcel bundles to).

Arguably, suffering through the slings and arrows of outrageous webpack is an important rite of passage, but my current life is better this way for sure.