Skip to content

Commit

Permalink
Merge pull request #106 from megatolya/patch-1
Browse files Browse the repository at this point in the history
Fix koa api example
  • Loading branch information
glenjamin authored Jun 15, 2016
2 parents 6239faf + ed6a80c commit 1ccbee7
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ var app = koa();
var hotMiddleware = require("webpack-hot-middleware")(compiler);
app.use(function* (next) {
yield hotMiddleware.bind(null, this.req, this.res);
yield hotMiddleware.bind(null, this.request, this.response);
yield next;
});
```
Expand Down

9 comments on commit 1ccbee7

@albertogasparin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@megatolya With this change I get the following error:

TypeError: res.writeHead is not a function
      at Object.handler (.../node_modules/webpack-hot-middleware/middleware.js:62:11)
      at middleware (.../node_modules/webpack-hot-middleware/middleware.js:40:17)
      at .../node_modules/co/index.js:136:8
      at new Promise (.../node_modules/core-js/modules/es6.promise.js:193:7)
      at Object.thunkToPromise (.../node_modules/co/index.js:135:10)
      at Object.toPromise (.../node_modules/co/index.js:119:55)
      at next (.../node_modules/co/index.js:99:29)
      at onFulfilled (.../node_modules/co/index.js:69:7)
      at .../node_modules/co/index.js:54:5
      at new Promise (.../node_modules/core-js/modules/es6.promise.js:193:7)

While hot mw is working perfectly fine with this.req & this.res.
Am I missing something? Is it working correctly for you?

@glenjamin
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From looking at http://koajs.in/doc/API/context/ it seems that request and response are the correct names to use with koa.

Perhaps you'll have more luck with https://github.com/leecade/koa-webpack-middleware which appears to be a wrapper around this module by someone who knows koa.

@albertogasparin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, request and response are the main Koa objects but they are a different thing from this.req and this.res. Indeed, your middleware needs the node response (not Koa enhanced one) and this.req/this.res are the correct object for the task.
I commented here to ask how @megatolya made your middleware work, as it does not look possible.

Besides, if we are just supposing that your middleware should work using this.request/this.response, I tell you right now that it is not, so I would revert the commit to restore the correct use with Koa.

@glenjamin
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in ffbc2d6

@megatolya
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi all!

I found that webpack-hot-middleware can't work with koa. And it should be strongly rewrited to support both frameworks.

@megatolya
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend to not support koa in this package

@albertogasparin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😶
Well, that sounds a bit too pessimistic. I have it successfully working in my projects, see https://github.com/albertogasparin/react-starterkit as an example. But I got it working with the "old" code, not with the changes you suggested.

@glenjamin
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest update in ffbc2d6 points to what appears to be a maintained wrapper lib by someone who actually uses koa, that's my "official" recommendation at this point - unless someone tells me otherwise.

I don't use koa, so I don't know enough to maintain any kind of support for it.

@albertogasparin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure, good compromise 👍

Please sign in to comment.