docs(guides): Add middleware section to dev guide #1481

TheDutchCoder · 2017-08-02T14:51:20Z

This PR adds the webpack-dev-middleware and webpack-hot-middleware to the Development Guide.

This PR adds the `webpack-dev-middleware` and `webpack-hot-middleware` to the Development Guide.

TheDutchCoder · 2017-08-02T14:52:41Z

@skipjack this adds quite a bit to the guide, and I hope it's sufficient. Didn't want to into more detail and overload this guide.

It was quite the challenge getting this to work, but I'm getting the hang of it ;)

Please check for consistency, spelling, formatting, and other things as usual. I probably looked over some stuff ;)

Will reference this PR in the stale PR that previously tried to add this section in.

skipjack

Left some minor comments in the first section. For the most part, it looks 🎉 . Thank you for tackling this and closing the other PR. By any chance did you add a demo to webpack-guides-code-examples repo? I'd be interested in playing around with it if you did.

As for the second section on HMR I would recommend dropping it from here (see my comment). Sorry, if I had realized you were planning to add that as well I would have said something earlier.

I think webpack-dev-middleware and webpack-hot-middleware are primarily for advanced usage so I think we should avoid digging too deep into them. Also, from what I just read in their readmes, the docs within those repos are actually pretty good so there's no need to repeat all of it.

skipjack · 2017-08-03T09:33:02Z

content/guides/development.md

@@ -240,7 +240,301 @@ T> Now that your server is working, you might want to give [Hot Module Replaceme

 ### Using webpack-dev-middleware

-?> Familiar with `webpack-dev-middleware`? We need your help! Please submit a PR to fill in the missing instructions and example here. Make sure to keep it simple as this guide is intended for beginners.
+`webpack-dev-middleware` is a wrapper that will emit files processed by webpack to a server. It's actually used in the `webpack-dev-server` itself, but if you want to use it with a different configuration (e.g. with `express`), you can use the middleqware.


/s/middleqware/middleware

Though maybe the whole thing can just be rephrased slightly...

webpack-dev-middleware is a wrapper that will emit files processed by webpack to a server. This is used by webpack-dev-server internally, however it's available as a separate package to allow more custom setups if desired. We'll take a look at an example that combines webpack-dev-middleware with an express server.

skipjack · 2017-08-03T09:45:05Z

content/guides/development.md

+__webpack.config.js__
+
+``` diff
+  var path = require('path');


I think we typically use const for node imports.

skipjack · 2017-08-03T09:46:52Z

content/guides/development.md

+
+``` diff
+  var path = require('path');
+  const htmlWebpackPlugin = require('html-webpack-plugin');


Not sure what the rest of the instances in this guide look like but in other guides we've capitalized plugin names (e.g. HtmlWebpackPlugin) and that seems to be the standard within the community for plugin imports. Personally, I actually capitalize all imports to delineate them from normal variables but that's a change I can PR later on if anything.

Completely right, typo on my end (rest was capitalized)

skipjack · 2017-08-03T09:47:12Z

content/guides/development.md

+    devtool: 'inline-source-map',
+    plugins: [
+      new CleanWebpackPlugin(['dist']),
+      new htmlWebpackPlugin({


Make sure to update this too.

skipjack · 2017-08-03T09:56:59Z

content/guides/development.md

+    output: {
+      filename: '[name].bundle.js',
+-     path: path.resolve(__dirname, 'dist')
+     path: '/',


This change will cause npm run build not to emit files to /dist anymore. webpack-dev-middleware manages output in memory as opposed to actually emitting files to output.path which is why when you run webpack-dev-server nothing in /dist is ever built or updated. I think what you want is something more along the lines of:

output: { filename: '[name].bundle.js' path: path.resolve(__dirname, 'dist'), publicPath: '/' // To serve from `localhost:3000` or '/app' to serve from `localhost:3000/app` }

I'm not 100% on this as I haven't used webpack-dev-middleware much, aside from behind the scenes in webpack-dev-server so I may have to test a bit. However, I do think that changing path will have unintended side effects for the rest of the npm scripts.

I'll need to test this, good point though!

You were completely right, will push the changes!

skipjack · 2017-08-03T09:59:53Z

content/guides/development.md

+  |- /src
+    |- index.js
+    |- print.js
+   |- server.js


You might want to actually place this outside of /src as it's not really part of the actual web application that's being deployed. Typically I've seen these kinds of scripts either in the root directory, or a separate /scripts directory though I think the root would be best here.

skipjack · 2017-08-03T10:05:35Z

content/guides/development.md

+      "watch": "webpack --progress --watch",
+      "start": "webpack-dev-server --open",
+     "server": "node src/server.js",
+      "build": "webpack"


This diff looks a bit funny, not sure if you see it the same way?

Is that indicating tabs were used instead of spaces or something?

Also, remember to update the server script to "node server.js" if you make the change I suggested above.

Yeah looks like tabs, will correct

skipjack · 2017-08-03T10:11:06Z

content/guides/development.md

+Now fire up your browser and go to `http://localhost:3000/app`, you should see your webpack app running and functioning!
+
+
+#### Adding Hot Module Replacement


I would drop this whole section -- if anything just make a note of the webpack-hot-middleware package at the end of the section above and link to it. Or, even better, just add a small tip to the Hot Module Replacement guide that notes and links to it, maybe something like:

T> If you took the route of using webpack-dev-middleware instead of webpack-dev-server, please use the webpack-hot-middleware package to enable HMR on your custom server or application.

Okay no problem, it was good practice! 😄

TheDutchCoder · 2017-08-03T14:58:43Z

Thanks @skipjack I will go through your feedback and make the appropriate changes hopefully this week! 👍

TheDutchCoder · 2017-08-04T17:43:15Z

@skipjack I think I resolved all issues/feedback, another look would be appreciated!

skipjack

A few minor things, aside from that this looks great!

skipjack · 2017-08-05T17:54:29Z

content/guides/development.md

+    output: {
+      filename: '[name].bundle.js',
+      path: path.resolve(__dirname, 'dist'),
+     publicPath: 'http://localhost/'


I would try just publicPath: '/' but if that didn't work then I guess this is fine. Frankly, I still find publicPath a bit confusing -- it almost seems like for fully qualified urls the first section, e.g. http://localhost is just ignored for generated bundles. I think maybe people use fully qualified URLs like https://cdn.example.com/ so that if something not generated by webpack returns a 404, the dev-server will fall back to actual location.

Yeah that works, so we should be good with that change!

skipjack · 2017-08-05T17:56:51Z

content/guides/development.md

+
+// Serve the files on port 3000.
+app.listen(3000, function () {
+  console.log('Example app listening on port 3000!');


Maybe throw a line break at the end of the statement so there's a little space between your and webpack's, e.g.:

console.log('Example app listening on port 3000!\n');

to generate below:

Example app listening on port 3000! webpack built 27b137af6d9d8668c373 in 1198ms Hash: 27b137af6d9d8668c373 ...

(somewhat nitpicky, I know -- feel free to ignore 😆 )

skipjack · 2017-08-05T17:57:41Z

content/guides/development.md

+
+Now fire up your browser and go to `http://localhost:3000`, you should see your webpack app running and functioning!
+
+T> If you took the route of using `webpack-dev-middleware` instead of `webpack-dev-server`, please use the [`webpack-hot-middleware`](https://github.com/glenjamin/webpack-hot-middleware) package to enable HMR on your custom server or application.


I meant add this somewhere in guides/hot-module-replacement.md... just pick a place in there that seems appropriate.

You want that tip to be added to the HMR guide instead and take it out of this one?

I can do that, just wanted to be sure I understood correctly ;)

TheDutchCoder · 2017-08-07T12:47:09Z

@skipjack This should cover it! ;)

skipjack

Looks great, thanks for making the changes! I made one change to the paragraph after publicPath to clarify but aside from that this was perfect. As soon as the build passes I'll merge.

TheDutchCoder · 2017-08-07T13:40:15Z

🎉

skipjack · 2017-08-07T17:46:13Z

Ok I'm going to merge as the build errors are just stemming from img.shields.io which...

should be fixed soon in General Updates #1491
and are not related to this PR

If any other issues come up from merging this (e.g. linting) then we can resolve in a separate PR.

docs(guides): Add middleware section to dev guide

1a58a80

This PR adds the `webpack-dev-middleware` and `webpack-hot-middleware` to the Development Guide.

TheDutchCoder requested a review from skipjack August 2, 2017 14:51

TheDutchCoder mentioned this pull request Aug 2, 2017

docs(guides): add using-webpack-dev-middleware section #1441

Closed

TheDutchCoder added 2 commits August 2, 2017 13:45

Fixed broken link

3c613ea

Added language to code block

d9abfb8

skipjack added Guides Enhancement labels Aug 3, 2017

skipjack requested changes Aug 3, 2017

View reviewed changes

TheDutchCoder added 2 commits August 4, 2017 13:35

First revisions based on feedback

f6d22f9

Updated the server/webpack config

35a1c74

Merge branch 'master' into TheDutchCoder-patch-1

ae08723

skipjack requested changes Aug 5, 2017

View reviewed changes

TheDutchCoder and others added 4 commits August 7, 2017 08:44

Minor adjustments

8d86a67

Removed tip

d7ae21d

Added webpack-dev-middleare tip

f64f477

Merge branch 'master' into TheDutchCoder-patch-1

10e0e96

docs(guides): update paragraph in development for clarity

942d216

skipjack approved these changes Aug 7, 2017

View reviewed changes

Merge branch 'master' into TheDutchCoder-patch-1

4e2f437

skipjack merged commit 4d42ac8 into master Aug 7, 2017

skipjack deleted the TheDutchCoder-patch-1 branch August 7, 2017 17:48

		Now fire up your browser and go to `http://localhost:3000/app`, you should see your webpack app running and functioning!


		#### Adding Hot Module Replacement


		Now fire up your browser and go to `http://localhost:3000`, you should see your webpack app running and functioning!

		T> If you took the route of using `webpack-dev-middleware` instead of `webpack-dev-server`, please use the [`webpack-hot-middleware`](https://github.com/glenjamin/webpack-hot-middleware) package to enable HMR on your custom server or application.

Uh oh!

docs(guides): Add middleware section to dev guide #1481

docs(guides): Add middleware section to dev guide #1481

Uh oh!

Conversation

TheDutchCoder commented Aug 2, 2017

Uh oh!

TheDutchCoder commented Aug 2, 2017

Uh oh!

skipjack left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheDutchCoder commented Aug 3, 2017

Uh oh!

TheDutchCoder commented Aug 4, 2017

Uh oh!

skipjack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheDutchCoder commented Aug 7, 2017

Uh oh!

skipjack left a comment

Choose a reason for hiding this comment

Uh oh!

TheDutchCoder commented Aug 7, 2017

Uh oh!

skipjack commented Aug 7, 2017

Uh oh!

Uh oh!

skipjack left a comment •

edited

Loading