fix: response correctly when receive an OPTIONS request#4185
fix: response correctly when receive an OPTIONS request#4185alexander-akait merged 4 commits intowebpack:masterfrom
Conversation
|
I try to run the test on my local machine. But it always hangs whenever it goes to the following steps. RUNS test/cli/basic.test.js |
Codecov Report
@@ Coverage Diff @@
## master #4185 +/- ##
==========================================
- Coverage 92.20% 92.11% -0.09%
==========================================
Files 16 16
Lines 1629 1637 +8
Branches 615 616 +1
==========================================
+ Hits 1502 1508 +6
- Misses 116 118 +2
Partials 11 11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Make sure you have linked
|
Oh, i didn't run this command. I'm sorry about i didn't read CONTRIBUTING.md carefully. |
|
@alexander-akait, any way we can get this in? |
|
@alan-agius4 Do you want this as built-in feature? |
|
Ideally, the dev-server can handle preflight requests out-of-the-box as it did in version 3. |
|
@alan-agius 4 interesting, because previously we don't handle it (i.e. we don't have code for this), it means one from middlewares handle this, I am not agait this merge, just want to check... |
|
@alexander-akait understood. |
|
@alan-agius4 Do you have time to check it (just a little busy right now), thank you |
|
Please let me know if some code needs to be changed, i would be happy to change it. |
|
a lot of differences in |
|
The OPTIONS request/response is handled by the express router https://github.com/expressjs/express/blob/master/lib/router/index.js#L164-L169 In version 4 however this check is always falsey, which causes the options not be handled properly. In version 3 this worked because of the additional express Layer that caused the options to be handled. setupMagicHtmlFeature() {
this.app.get('*', this.serveMagicHtml.bind(this));
}This is because the above cause a creation of an additional layer where In fact adding the below in app.get('*', (req, res, next) => { next(); }); |
|
@alan-agius4 thank for this, I think we can enable it by default, should be safe for dev env @TrickyPi Can you enable it by default? |
|
Ok |
For Bugs and Features; did you add new tests?
Yes
Motivation / Use-Case
fix: #4180
Breaking Changes
No
Additional Info
No