Change proxy handling to allow multiple proxies in development

[jamesblight]

This is an attempt to resolve #1780 by using the proxy handling of Webpack dev server to allow multiple proxies to be added to package.json.
Supported config options can be found here: https://webpack.github.io/docs/webpack-dev-server.html#proxy

The changes are compatible with the previous method and behaviour of providing a simple proxy string

{
    "proxy": "http://localhost:4000"
}

I've tested the following proxy scenarios:

Proxy as a string

Expected behaviour is maintained. Requests that accept text/html always fallback to /index.html

Proxy as an object in the form:

{
    "proxy": { "<context>": { "target": "<url>" } }
}

All matching requests are proxied to the target. This includes requests with a 'text/html' Accept Header. It is up to the user to bypass those requests manually in the config.

No proxy specified in package.json

All requests fallback to /index.html

This change relies on Webpack dev server being updated to handle the external websocket upgrade as CRA no longer adds the proxy middleware.

This is a step towards #671
#1702 Should be solved by adding a custom oauth callback proxy

EDIT:
Webpack dev server v2.4.2 handles the external websocket upgrade for all proxies.
webpack/webpack-dev-server#843

Please provide any feedback.

[SpaceK33z]

Awesome, thanks for picking this up!

This change relies on Webpack dev server being updated to handle the external websocket upgrade as CRA no longer adds the proxy middleware.

Were you planning on making a PR for webpack-dev-server? If so I'll hold off on releasing the next version for a bit and review it soon 👍.

[jamesblight]

@SpaceK33z I've made the change for WDS here. I need to verify that it's working with some tests. The change will upgrade every websocket proxy, are you ok with that approach?

[SpaceK33z]

@jamesblight great, yeah I think that makes sense.

[Zn4rK]

My favicon.ico is forced into the history fallback if I'm not using htmlAcceptHeaders: ['text/html'].

Edit: I don't really know if that is what's happening, but theres something weird going on when specifing the proxy in either way

Switching Routes using the Proxy Object:

Switching Routes using the Proxy String:

I might have goofed something up when trying this PR out, but maybe someone could confirm if they are seeing the same?

[jamesblight]

Thanks for reporting this, I'll have to investigate to see what's happening. disableDotRule is used in the original proxy implementation and there's obviously a conflict when using the proxy object.

[Zn4rK]

I think there's a conflict with the proxy string as well. I don't think there should be more than the one request the browser normally does for the favicon in a default setup.

Switching routes without proxy:

[jamesblight]

Would you be able to create a minimal project reproducing this? I can't replicate using the standard CRA app.

[Zn4rK]

Sure, but while I was setting up the minimal project I noticed that I couldn't reproduce it with that. So I decided to try again with my real project - and this time around I couldn't make the above happen. I don't really know what was going on, but it was definitely something strange. Sorry about that!

I'll investigate a bit more, and get back to you with a minimal project if I can reproduce again. But I basically used https://reacttraining.com/react-router/web/guides/quick-start to make it happen the first time.

[nordhagen]

Any progress on this?

[jamesblight]

@nordhagen are you able to confirm that this branch fixes #1702?

[Timer]

I would love to see some more bikeshedding done on this issue. Let me know if you guys need anything from me.

[Tanhe]

Is there anything I could do to help this along? This feature would really make my life easier. I'm currently forced to run a reverse proxy in front of my react-scripts app to bring in a shared login application running in a pre-production environment.

[Timer]

I'll tag this so it doesn't slip through the cracks, but I want to make sure the schematics are final first.

@Tanhe if you try this PR can you please let me know if there's anything you'd change?

[Tanhe]

Tried to test this but couldn't create a new app with the PR as I'm on Windows (looks like npm run create-react-app uses a shell script...).

The code looks good though. It maintains backwards compatibility when proxy is a string, and adds support for providing a webpack-dev-server config object directly.

This could be taken a step further to also support specifying a function which returns the configuration. I know that I often have to use different configurations under different scenarios and this would only add a minor complication to what has already been changed.

Question...when I "npm start" from the create-react-app checkout...what package.json and/or src is it serving? I would test this way if it's possible.

[Timer]

Tried to test this but couldn't create a new app with the PR as I'm on Windows (looks like npm run create-react-app uses a shell script...).

It should work regardless in powershell or similar.

Question...when I "npm start" from the create-react-app checkout...what package.json and/or src is it serving? I would test this way if it's possible.

packages/react-scripts/

[Tanhe]

Was able to test this PR and it works as advertised. Both for the existing proxy as a string setting and the proxy as an object case.

When specifying as an object it does result in the development webserver logging one message per configured proxy:

[HPM] Proxy created: function (pathname) {
          return mayProxy.test(pathname) && pathname.match(context);
        }  ->  https://example.com
[HPM] Proxy created: function (pathname) {
          return mayProxy.test(pathname) && pathname.match(context);
        }  ->  http://example.com:5511

[draperd]

Is there anything I can do to help with this at all? It's a feature I'd really like to see added.

[draperd]

I've pulled the branch from @jamesblight and have successfully tested it locally with 2 proxies configured. Is the failing CI integration test related to the Yarn issue I found in #2072 ? (see #2072 (comment))

[jamesblight]

I resolved the conflicts caused by #1839, but I haven't tested this on windows. @draperd Could you pull the latest branch and test again? #1839 introduced an asynchronous dns lookup for windows which changed the proxy handling.

[draperd]

@jamesblight I've pulled your latest branch, built a new app with 2 proxies configured to different servers with different REST APIs and tested again and it all works fine for me.

Thanks for doing this - it's working really well, hopefully it can get merged!

[gaearon]

I updated this to master. However now it fails when we open 0.0.0.0 (which we default to):

Invalid Host header

This brings me to a WDS issue: webpack/webpack-dev-server#882.

I worked around it by setting disableHostCheck: true but it doesn’t feel great. Does anybody understand what this is referring to, and how proxies affect it?

[gaearon]

Okay, I fixed a bug here, but we realized we have a bug on master too related to disableHostCheck.
This PR should probably be okay to land though.

[gaearon]

@Timer I'd appreciate if you can jump on this, move prepareProxy() into react-dev-utils, do some local testing and get it in. We have other problems on master (port clash and 0.0.0.0 problem) but those would be better addressed on top.

[Timer]

This is working well for me now.

[Timer]

Let's hope this doesn't blow up. 😄 All seems well on mac & windows, however. CI is passing on my branch.

Sliding this in because we need to build on it.

[gaearon]

Please help beta test the new version that includes this change!
#2172

[shellscape]

@Timer @gaearon apologies for resurrecting a long-dead thread, but please see my comment here regarding WebSocket proxying and the new version of webpack-dev-server in development. That comment's contents may impact create-react-app, and I sure would love to get any assistance on that, not entire sure that @jamesblight is still active around these parts.