-
-
Notifications
You must be signed in to change notification settings - Fork 376
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(middleware): change how mime type option is handled #670
fix(middleware): change how mime type option is handled #670
Conversation
Codecov Report
@@ Coverage Diff @@
## master #670 +/- ##
=======================================
Coverage 99.11% 99.11%
=======================================
Files 8 8
Lines 227 227
Branches 70 70
=======================================
Hits 225 225
Misses 2 2
Continue to review full report at Codecov.
|
content = handleRangeHeaders(content, req, res); | ||
// Content-Type and headers need to be set before checking if | ||
// the file is in the outputFileSystem, as these things should be | ||
// applied to all files that are being served |
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.
correct me if this approach is incorrect, but it seems that headers and content types need to be applied before rejecting a file that is not in the outputFileSystem. This is because files/routes served by webpack-dev-server like localhost:8080/main
, or files that are in the user's directory and are being served, are not in the middleware filesystem but should still have the headers and content types applied to them. That is what the previous behavior was. But let me know if this was an intentional breaking change.
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.
Yes, looks like regression
/cc @hiroppy |
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.
lgtm
@Loonride should we do patch release? Or we have more regressions? |
@evilebottnawi Not yet, there still may be more that needs to be done. Still working through integrating it on the webpack-dev-server side |
👍 |
@evilebottnawi I think it is good now for a patch release |
This PR contains a:
Motivation / Use-Case
I think that the
mimeType
option should take priority over existing, known mime types. Otherwise, specifying amimeType
for something like html or js will have no effect.Breaking Changes
Change of mimeType option priority with existing mime types
Additional Info