-
Notifications
You must be signed in to change notification settings - Fork 32
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
Try to serve static before rewrites when using serve option #236
Try to serve static before rewrites when using serve option #236
Conversation
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
==========================================
+ Coverage 52.6% 52.83% +0.23%
==========================================
Files 10 10
Lines 614 617 +3
Branches 154 154
==========================================
+ Hits 323 326 +3
Misses 291 291
Continue to review full report at Codecov.
|
} else { | ||
app.use(express.static(outputDir)); | ||
} | ||
serveStatic(app, outputDir, args.mode, args.compression); |
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.
This is the second time serveStatic
is called within serve
, without any return
or conditional in between. Is this deliberate?
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.
It's is, the first time is before the history middleware and the middleware doesn't call next if it successfully serve a resource. If it doesn't find a resource then it will carry on with the middleware and run the history re-write middleware and then try and serve the re-written resource.
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 should probably rename the function to either 'use....' or 'register....'
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.
Ah, that makes sense. Thanks for the explanation.
} else { | ||
app.use(express.static(outputDir)); | ||
} | ||
serveStatic(app, outputDir, args.mode, args.compression); |
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.
Ah, that makes sense. Thanks for the explanation.
Type: bug
The following has been addressed in the PR:
prettier
Description:
When using BTR with
StateHistory
anindex.html
is written for each path configured, so when usingserve
option it shouldn't automatically rewrite index requests to the mainindex.html
. This changes the middleware to try and serve files before re-writing the URL and only re-writes the URL when the index file does not exist.