Fix proxy bypass return false#1696
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1696 +/- ##
==========================================
+ Coverage 83.76% 83.85% +0.09%
==========================================
Files 8 8
Lines 536 539 +3
Branches 161 162 +1
==========================================
+ Hits 449 452 +3
Misses 70 70
Partials 17 17
Continue to review full report at Codecov.
|
alexander-akait
left a comment
alexander-akait
left a comment
There was a problem hiding this comment.
Looks good, just question
JakobJingleheimer
left a comment
JakobJingleheimer
left a comment
There was a problem hiding this comment.
The suggested change (multiple nested ternaries and ifs) is too convoluted and has no code comments to help explain. No-one looking at this 5 minutes later would understand it 😨
|
@evilebottnawi @jshado1 thanks for the suggestions. I tried to make the logic more clear and simple. |
…pack-dev-server into fix-proxy-bypass-return-false
|
@evilebottnawi I think you can merge this 😄 |
| ? proxyConfig.bypass(req, res, proxyConfig) | ||
| : null; | ||
|
|
||
| if (typeof bypassUrl === 'boolean') { |
There was a problem hiding this comment.
This is not aligning to the documentation
In the function you get access to the request, response and proxy options. It must return either false or a path that will be served instead of continuing to proxy the request.
There was a problem hiding this comment.
PR welcome to webpack docs
There was a problem hiding this comment.
6 participants
For Bugs and Features; did you add new tests?
Yes
Motivation / Use-Case
Based on the explanation in the docs
It was expected that
return falseskip the proxy with a 404Breaking Changes
None
Additional Info
Closes #1677
/CC @evilebottnawi
@jsmith-sapient @jshado1 @evilebottnawi could you please test this manually and confirm that this have the intended behaviour?