-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Change proxy handling to allow multiple proxies in development #1790
Merged
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
2f79303
Change proxy handling to allow multiple proxies to be specified in pa…
jamesblight 6f4dd69
Up webpack-dev-server to 2.4.2
jamesblight e859ec9
Merge branch 'master' into multiple-proxies
jamesblight c5c2d36
Merge branch 'master' into jamesblight-multiple-proxies
gaearon a1d9c31
Fix the listen() call
gaearon 226088b
Switch to correct default host
Timer 2e099d8
Remove promises and extract to react-dev-utils
Timer 6c1436e
Merge branch 'master' into multiple-proxies
Timer 9e855fd
oops
Timer 15688e7
Merge branch 'master' into multiple-proxies
Timer fad7d3e
Merge branch 'master' into multiple-proxies
Timer 4198c8c
Merge branch 'master' into multiple-proxies
Timer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
132 changes: 0 additions & 132 deletions
132
packages/react-scripts/scripts/utils/addWebpackMiddleware.js
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
// @remove-on-eject-begin | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
// @remove-on-eject-end | ||
'use strict'; | ||
|
||
const chalk = require('chalk'); | ||
|
||
// We need to provide a custom onError function for httpProxyMiddleware. | ||
// It allows us to log custom error messages on the console. | ||
function onProxyError(proxy) { | ||
return (err, req, res) => { | ||
const host = req.headers && req.headers.host; | ||
console.log( | ||
chalk.red('Proxy error:') + | ||
' Could not proxy request ' + | ||
chalk.cyan(req.url) + | ||
' from ' + | ||
chalk.cyan(host) + | ||
' to ' + | ||
chalk.cyan(proxy) + | ||
'.' | ||
); | ||
console.log( | ||
'See https://nodejs.org/api/errors.html#errors_common_system_errors for more information (' + | ||
chalk.cyan(err.code) + | ||
').' | ||
); | ||
console.log(); | ||
|
||
// And immediately send the proper error response to the client. | ||
// Otherwise, the request will eventually timeout with ERR_EMPTY_RESPONSE on the client side. | ||
if (res.writeHead && !res.headersSent) { | ||
res.writeHead(500); | ||
} | ||
res.end( | ||
'Proxy error: Could not proxy request ' + | ||
req.url + | ||
' from ' + | ||
host + | ||
' to ' + | ||
proxy + | ||
' (' + | ||
err.code + | ||
').' | ||
); | ||
}; | ||
} | ||
|
||
module.exports = function prepareProxy(proxy) { | ||
// `proxy` lets you specify alternate servers for specific requests. | ||
// It can either be a string or an object conforming to the Webpack dev server proxy configuration | ||
// https://webpack.github.io/docs/webpack-dev-server.html | ||
if (proxy) { | ||
if (typeof proxy !== 'object' && typeof proxy !== 'string') { | ||
console.log( | ||
chalk.red( | ||
'When specified, "proxy" in package.json must be a string or an object.' | ||
) | ||
); | ||
console.log( | ||
chalk.red('Instead, the type of "proxy" was "' + typeof proxy + '".') | ||
); | ||
console.log( | ||
chalk.red( | ||
'Either remove "proxy" from package.json, or make it an object.' | ||
) | ||
); | ||
process.exit(1); | ||
} | ||
|
||
// Otherwise, if proxy is specified, we will let it handle any request. | ||
// There are a few exceptions which we won't send to the proxy: | ||
// - /index.html (served as HTML5 history API fallback) | ||
// - /*.hot-update.json (WebpackDevServer uses this too for hot reloading) | ||
// - /sockjs-node/* (WebpackDevServer uses this for hot reloading) | ||
// Tip: use https://jex.im/regulex/ to visualize the regex | ||
const mayProxy = /^(?!\/(index\.html$|.*\.hot-update\.json$|sockjs-node\/)).*$/; | ||
|
||
// Support proxy as a string for those who are using the simple proxy option | ||
if (typeof proxy === 'string') { | ||
return [ | ||
{ | ||
target: proxy, | ||
logLevel: 'silent', | ||
// For single page apps, we generally want to fallback to /index.html. | ||
// However we also want to respect `proxy` for API calls. | ||
// So if `proxy` is specified as a string, we need to decide which fallback to use. | ||
// We use a heuristic: if request `accept`s text/html, we pick /index.html. | ||
// Modern browsers include text/html into `accept` header when navigating. | ||
// However API calls like `fetch()` won’t generally accept text/html. | ||
// If this heuristic doesn’t work well for you, use a custom `proxy` object. | ||
context: function(pathname, req) { | ||
return mayProxy.test(pathname) && | ||
req.headers.accept && | ||
req.headers.accept.indexOf('text/html') === -1; | ||
}, | ||
onProxyReq: proxyReq => { | ||
// Browers may send Origin headers even with same-origin | ||
// requests. To prevent CORS issues, we have to change | ||
// the Origin to match the target URL. | ||
if (proxyReq.getHeader('origin')) { | ||
proxyReq.setHeader('origin', proxy); | ||
} | ||
}, | ||
onError: onProxyError(proxy), | ||
secure: false, | ||
changeOrigin: true, | ||
ws: true, | ||
xfwd: true, | ||
}, | ||
]; | ||
} | ||
|
||
// Otherwise, proxy is an object so create an array of proxies to pass to webpackDevServer | ||
return Object.keys(proxy).map(function(context) { | ||
if (!proxy[context].hasOwnProperty('target')) { | ||
console.log( | ||
chalk.red( | ||
'When `proxy` in package.json is as an object, each `context` object must have a ' + | ||
'`target` property specified as a url string' | ||
) | ||
); | ||
process.exit(1); | ||
} | ||
|
||
return Object.assign({}, proxy[context], { | ||
context: function(pathname) { | ||
return mayProxy.test(pathname) && pathname.match(context); | ||
}, | ||
onProxyReq: proxyReq => { | ||
// Browers may send Origin headers even with same-origin | ||
// requests. To prevent CORS issues, we have to change | ||
// the Origin to match the target URL. | ||
if (proxyReq.getHeader('origin')) { | ||
proxyReq.setHeader('origin', proxy[context].target); | ||
} | ||
}, | ||
onError: onProxyError(proxy[context].target), | ||
}); | ||
}); | ||
} | ||
}; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My
favicon.ico
is forced into the history fallback if I'm not usinghtmlAcceptHeaders: ['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](https://cloud.githubusercontent.com/assets/873043/23855077/94b51444-07f4-11e7-8f2d-c622bab88555.png)
Switching Routes using the Proxy String:
![switching-routes-proxy-string](https://cloud.githubusercontent.com/assets/873043/23855136/d4890558-07f4-11e7-859b-37e9f9243394.png)
I might have goofed something up when trying this PR out, but maybe someone could confirm if they are seeing the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
![switch-routes-default-setup](https://cloud.githubusercontent.com/assets/873043/23878650/025e17d0-0849-11e7-9eab-6dca329c25d9.png)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be able to create a minimal project reproducing this? I can't replicate using the standard CRA app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.