webpack-dev-server
webpack-dev-server copied to clipboard
devServer proxy /api/* is actually /api
- Operating System:
- Node Version:
- NPM Version:
- webpack Version:
- webpack-dev-server Version: 3.10.3
- Browser:
- [x] This is a bug
- [ ] This is a modification request
Code
When using the 'context': 'target' or 'context': { ...options... } style of proxy, if /* is at the end of the context it is removed.
https://github.com/webpack/webpack-dev-server/blob/50c09a4b64c013cca0acb6013bdaa28d0f342149/lib/Server.js#L230-L233
See discussion in #359 for why this was done
So, for a config such as this:
// webpack.config.js
module.exports = {
//...
devServer: {
proxy: {
'/api/*' : 'http://localhost:3000'
}
}
};
Expected Behavior
Proxies everything starting with /api/
i.e. /api/stuff but NOT /apistuff.html
Actual Behavior
Proxies everything starting with /api
including /apistuff.html
For Bugs; How can we reproduce the behavior?
- Create new project using The SAFE Template:
dotnet new SAFE - To make it clear when requests are being proxied to the server, replace
use_static publicPathwithuse_router (text "from server")insrc/Server/Server.fs - Run the project using
fake build -t run - Observe that localhost:8080/apistuff.html results in
from server, because it is being proxied to the server (whereas localhost:8080/notapistuff.html results inCannot GET /notapistuff.htmlbecause it is not being proxied)
For Features; What is the motivation and/or use-case for the feature?
N/A
Bug, thanks for the issue, feel free to send a PR
Happy to make a PR. I've got a couple different thoughts of how to approach fixing it.
I think the most important part about this issue that the behaviour is different between the two styles of proxy config.
It would be quite reasonable to expect that changing the config from
devServer: {
proxy: {
'/api/*' : 'http://localhost:3000'
}
}
to
devServer: {
proxy: [
{
context: '/api/*',
target: 'http://localhost:3000'
}
]
}
would not have an effect on the behaviour.
However, because the latter does not modify the proxyOptions before sending it HPM, it actually has the behaviour of proxying ONLY /api/*, and NOT for example /api or /api/v1/stuff.
Just not altering the context is not really an option because it would break backward compatibility (as seen in #359)
With that in mind, what do you think of the following options.
Instead of removing trailing /*, replace it with /**
This would result in the expected behaviour,
Proxies everything starting with /api/ i.e. /api/stuff but NOT /apistuff.html
but would not fix the discrepency between the two styles and so changing to the array style would still have an unexpected change
Deprecate the use of trailing '/*'
We may as well fix the discrepency with using a single '*' while we're at it.
Deprecate the use of '*' and traling '/*' in the 'context': 'target' style in favour of '**' and '/**'
Deprecate the use of the 'context': 'target' style altogether
Supporting only arrays and not properties would be a simplification of the code base.
Configurations that need extra config need to use the { ...options...} style anyway so may as well use { context: '/api', ...options... }
And for simple proxies that do not need extra configuration, we could add support for https://github.com/chimurai/http-proxy-middleware#shorthand
I think we should use logic as in express
Hey guys, is there an update on this? I'm having trouble proxying across federated modules (used to work before beta v4)
Not sure. I we can decide which suggested option to take (or something else) I'd still be happy to make a PR.
@vtrikoupis I'd suggest changing any trailing /* with /** or using the array style.
I think we can remove it for the next major release (beta now)
What do you mean by "remove it"? Remove what?
backwards compatibility for old package
So what exactly are you saying? Just simply remove these lines?
https://github.com/webpack/webpack-dev-server/blob/feb7eb044eb0b2cfe7b58d19ea9c35dd049053d0/lib/Server.js#L174-L175
That would be a breaking change and likely cause issues with a lot of people's configurations.
I think we can remove it for the next major release (beta now)
Yes I understand doing it in a major release but not everyone is going to read the release notes or understand the ramifications.
In this case we don't solve this problem, on the one hand we cannot do it, on the other we have to fix it
@alexander-akait have you considered the suggestions in https://github.com/webpack/webpack-dev-server/issues/2454#issuecomment-597044551 ?
hm, make sense, I think we should add deprecation message here, so in future we will drop it
Feel free to send a PR
Okay. Just to be clear, do you mean deprecating use of * and trailing /*, or deprecating the 'context': 'target' style altogether?
Ideally we should be as close as possible to http-proxy-middleware package, I think we should support this:
{
proxy: [
{
context: Filter | Options
options?: options?: Options
}
]
}
From node_modules/http-proxy-middleware/dist/index.d.ts
We really have a lot of complex stuff here...
I don't remember all cases (because I don't write them :smile: ), so we need be careful, but I agree, we need simplify it and remove unnecessary complex
Ideally we should as close as possible to
http-proxy-middlewarepackage, I think we should support this:{ proxy: [ { context: Filter | Options options?: options?: Options } ] }From
node_modules/http-proxy-middleware/dist/index.d.ts
Agreed. So it sounds like you reckon deprecate the object synax like
{
proxy: { '/api': 'http://localhost:3000' }
}
in favour of the full array syntax like http-proxy-middleware.
Yep, ideally we should avoid using object syntax, full array syntax like http-proxy-middleware is more flexibility and avoid misleading between options dev server and http-proxy-middleware package
Great. Will make a PR
Fixed in 5.0.0