Improved README.md #10
Conversation
README.md
Outdated
| server.register(require('fastify-http-proxy'), { | ||
| upstream, | ||
| prefix: '/upstream', // optional | ||
| 'http://my-api.example.com', |
There was a problem hiding this comment.
upstream: 'http://my-api.example.com'
README.md
Outdated
| const proxy = require('fastify-http-proxy') | ||
|
|
||
| server.register(proxy, { | ||
| 'http://my-api.example.com', |
There was a problem hiding this comment.
upstream: 'http://my-api.example.com'
README.md
Outdated
| }) | ||
|
|
||
| server.register(proxy, { | ||
| 'http://single-signon.example.com/auth', |
There was a problem hiding this comment.
upstream: 'http://my-api.example.com'
| For a more complete example, see `example.js`. | ||
| This will proxy any request starting with `/api` to `http://my-api.example.com`. For instance `http://localhost:3000/api/users` will be proxied to `http://my-api.example.com/users`. | ||
|
|
||
| If you want to have different proxies on different prefixes in you can register multiple instances of the plugin as shown in the following snippet: |
There was a problem hiding this comment.
If you register this plugin multiple times, probably the prefix handling should be required :)
There was a problem hiding this comment.
Good point, let me sure we capture this as it might be non-obvius!
There was a problem hiding this comment.
Let me know if you are ok with the clarification added with the latest commit
README.md
Outdated
| ``` | ||
|
|
||
| For a more complete example, see `example.js`. | ||
| Notice that in these case it is important to use the `prefix` option and to have *non-ambiguous* or *non-overlapping* prefixes (e.g. `/api` and `/api2`). |
There was a problem hiding this comment.
I think the non-overlapping part is a bug.
There was a problem hiding this comment.
@lmammino what happens if you use /api and /api2?
There was a problem hiding this comment.
I am actually not sure how prefixes are resolved and I haven't tested if this specific example is ambiguous.
Anyway, we could actually make this a little bit better by saying something like this:
Notice that in these case it is important to use the
prefixoption and to have non-ambiguous or non-overlapping prefixes (e.g. don't use/apiand/api2but/api/and/api2/).
I am assuming here the second case will behave properly, but maybe it's worth updating the tests to validate these cases.
What do you think @mcollina ? Any opinion from you @delvedor ?
There was a problem hiding this comment.
Since the routing is handled by find-my-way on our side, /api and /api2 are two completely different routes and the overlapping should not be a problem. Given that, a test has never killed anyone :P
There was a problem hiding this comment.
I am going to add a test and refactor the description if no issue is raised
|
Thank you sir @delvedor ! 🤩 |
Trying to improve a bit the quality of this readme with more descriptive examples and other little changes.