Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use http-proxy-middleware instead of http-proxy. #359

Merged
merged 2 commits into from
Jan 11, 2016

Conversation

jamsesso
Copy link
Contributor

@jamsesso jamsesso commented Jan 3, 2016

I've found that the configuration of the node-http-proxy (NHP) library doesn't allow for some basic use cases (including my own). For example, it doesn't seem possible (through configuration only) to proxy a request from host1.com/api/user/1 to host2.com/user/1.

This PR changes the implementation to use the http-proxy-middleware (HPM) library which is built on top of the original NHP, so all current configurations should work. The HPM library has a lot more configuration options available.

I have ported over proxyOptions.bypass(...), but without a proxy object, there is no need for proxyOptions.configure(...) anymore.

Comments in the code can be removed on request, I wanted to make my intent clear. Cheers

sokra added a commit that referenced this pull request Jan 11, 2016
Use http-proxy-middleware instead of http-proxy.
@sokra sokra merged commit cf091e2 into webpack:master Jan 11, 2016
@sokra
Copy link
Member

sokra commented Jan 11, 2016

Thanks

@mjrussell
Copy link
Contributor

👍 for this PR. It adds support for native web sockets which is currently not working in the latest release. Would resolve #283. Also seems like the HTPM has much more mature documentation.

One issue I did have though is that the syntax for describing the url to match is a little different.

Before this PR:

  ...
  proxy: {
    '/api/*': {
       ...
    }
  }
  ...

After:

  ...
  proxy: {
    '/api': {
       ...
    }
  }
  ...

@mjrussell
Copy link
Contributor

@sokra Should there be some documentation regarding the proxy path tweak?

@mjrussell
Copy link
Contributor

Also I noticed a crash when the server being proxied to shuts down, opened an issue with the HTPM since the trace looks like its on them

@jamsesso
Copy link
Contributor Author

Thanks for the merge. If there is a documentation repository I can make the changes for the proxy path tweak. I tested the /api/* path and I am pretty sure it worked. Can you verify that it is not working before the documentation is updated?

@mjrussell
Copy link
Contributor

I did run it on my setup and it was definitely not obeying the /api/*. The docs on the npm registry seem to suggest you should just use /api.

I don't have a great open source example for a demo on hand. @sokra added a rewrite in 33ebd1f. If you can prove that this is unnecessary feel free to and then that check can be removed.

@jamsesso jamsesso mentioned this pull request Jan 12, 2016
@unindented
Copy link

@sokra What's the schedule for webpack-dev-server releases? I'd love to get this on npm.

(I can't install it straight from git because npm doesn't seem to run the prepublish tasks.)

@mik01aj
Copy link

mik01aj commented Jan 22, 2016

+1, I wait for the npm release.

EDIT: Ah, nevermind, I noticed 2.0.0-beta published yesterday.

P.S. It would be nice if there was some changelog or upgrade guide for 2.0.0.

@geddski
Copy link

geddski commented Jun 21, 2016

@mik01aj the 1.14.1 release has it.

saifelse pushed a commit to benchling/webpack-dev-server that referenced this pull request Jul 19, 2016
Use http-proxy-middleware instead of http-proxy.
saifelse pushed a commit to benchling/webpack-dev-server that referenced this pull request Jul 19, 2016
SpaceK33z added a commit that referenced this pull request Aug 23, 2016
The `bypass` feature was no longer working because the code added the bypass method as middleware AND `httpProxyMiddleware` as middleware. However, if a `bypass` method is given, it should only use that as middleware.

This was caused in #359, and effects `1.15.0` and the 2.x branch.
SpaceK33z added a commit that referenced this pull request Aug 23, 2016
The `bypass` feature was no longer working because the code added the bypass method as middleware AND `httpProxyMiddleware` as middleware. However, if a `bypass` method is given, it should only use that as middleware.

This was caused in #359, and effects `1.15.0` and the 2.x branch.
SpaceK33z added a commit that referenced this pull request Aug 23, 2016
The `bypass` feature was no longer working because the code added the bypass method as middleware AND `httpProxyMiddleware` as middleware. However, if a `bypass` method is given, it should only use that as middleware.

This was caused in #359, and effects `1.15.0` and the 2.x branch.
SpaceK33z added a commit that referenced this pull request Aug 23, 2016
The `bypass` feature was no longer working because the code added the bypass method as middleware AND `httpProxyMiddleware` as middleware. However, if a `bypass` method is given, it should only use that as middleware.

This was caused in #359, and effects `1.15.0` and the 2.x branch.
SpaceK33z added a commit that referenced this pull request Aug 24, 2016
Apparently this whole config option was removed in PR #359.
@sinedied
Copy link

Any guidance on how to make this work behind a corporate http proxy?
In my old gulp-based setup I could setup a custom agent to do this (via http-proxy-middleware agent option), how can I do the same here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants