Simplify proxy addon example #62
Conversation
Codecov Report
@@ Coverage Diff @@
## master #62 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 7 7
Lines 269 269
=======================================
Hits 248 248
Misses 21 21Continue to review full report at Codecov.
|
|
Thanks for the PR 🍺 I'm good with an additional example, but there's no practical reason to remove information here. The original example should stay. If you'd like to move you changes into |
|
What confuses me the most, about the original example, is why have a router at all? The proxy middleware does its own URL checks, what does the router do? And, related to that, why it "must be the last middleware added"? The app can function fine while having the proxy as the first middleware in the chain, as long as it isn't catch-all. I think the main proxy example should be about proxying |
|
Please see my previous comment; That's what I'll accept in a PR at this time. |
| }; | ||
|
|
||
| // This add-on will route all incoming requests for | ||
| // "http://localhost:8080/api/*" to "http://localhost:8081/api/*" (note the port |
There was a problem hiding this comment.
Well, it may be something other than 8080, but decided to go for brevity.
| module.exports.serve = { | ||
| content: [__dirname], | ||
| add: (app, middleware, options) => { | ||
| app.use(convert(proxy('/api', { target: 'http://localhost:8081' }))); |
There was a problem hiding this comment.
Inline options, to make it easier to copy-paste.
| @@ -386,6 +386,7 @@ Listed below are some of the add-on patterns and recipes that can be found in | |||
| - [compress](docs/addons/compress) | |||
| - [historyApiFallback](docs/addons/history-fallback.config.js) | |||
| - [proxy](docs/addons/proxy.config.js) | |||
| - [spa-with-api-simple](docs/addons/spa-with-api-simple.config.js) | |||
There was a problem hiding this comment.
Naming is hard, but seems self-explanatory and catchy enough.
| // applications. | ||
| // | ||
| // To remove the "/api" prefix when proxying the API requests, just add | ||
| // "pathRewrite: { '^/api': '' }" to proxy's options. |
There was a problem hiding this comment.
I think this is common enough to be worth a protip.
|
Cheers 🍺 |
This PR contains a:
Motivation / Use-Case
Updated the proxy addon example to be closer to what people probably use it for in the context of
webpack-serve/webpack-dev-server. Routing usually isn't needed withhttp-proxy, and when it is,koa-mountwould probably be a better fit.Additional Info
I think a combined example of HTTP proxy + history API fallback is warranted, but I want to know whether you think it's needed or not first, before adding it.
Also, would be nice to have all examples
koa-connect-free. I can see why they aren't right now, the existing Koa 2 equivalent libs are a bit sub par, but that's addressable.