Fix/issue 463 pretty urls in prodServer #464
Conversation
hutchgrant
changed the title
thescientist13
added
the
enhancement
packages/cli/src/lifecycles/serve.js
Outdated
| @@ -69,7 +69,9 @@ function getProdServer(compilation) { | |||
| const { outputDir } = compilation.context; | |||
|
|
|||
| if (ctx.request.url.endsWith('/')) { | |||
| ctx.redirect(`http://localhost:8080${ctx.request.url}index.html`); | |||
| const contents = await fsp.readFile(path.join(outputDir, ctx.request.url, 'index.html'), 'utf-8'); | |||
There was a problem hiding this comment.
Does combining the / and .html tests not work well together?
if (ctx.request.url.endsWith('/')) {
const contents = await fsp.readFile(path.join(outputDir, ctx.request.url, 'index.html'), 'utf-8');
ctx.set('Content-Type', 'text/html');
ctx.body = contents;
}Or I suppose it could also make sense to keep .html as a stand alone handler as well?
There was a problem hiding this comment.
right I think need to keep it in case?
There was a problem hiding this comment.
right I think need to keep it in case?
in case of what though? I wasn't sure if you had a case in mind, so the question was mostly for you. ;)
What cases do we have of needing to process .html differently? I could at least see treating both with the same logic perhaps?
if (ctx.request.url.endsWith('/') || ctx.request.url.endsWith('.html')) {
const barePath = url.endsWith('/') ? '' : ctx.request.url;
const contents = await fsp.readFile(path.join(outputDir, barePath, 'index.html'), 'utf-8');
ctx.set('Content-Type', 'text/html');
ctx.body = contents;
}Mostly just spitballing at this point, so no big deal.
There was a problem hiding this comment.
lets go with that then. I don't have an immediate use case are we resolving all custom html files to their own routes and prettifying them ? e.g. docs.html would just be /docs/ ? I don't know I havent tested that yet.
There was a problem hiding this comment.
yeah, give it a test. At the end of the day they both return the contents of an HTML file, it's just that we can do it simply without including a redirect in either case.
|
did a quick refactor as well. |
* task: remove the url redirect from serve lifecycle * fix: remove redundant case * fix: accomodate present files with .html in pathname * fix: typo * fix: correct path * fix: combine html/prettified url case * fix: small refactor
* task: remove the url redirect from serve lifecycle * fix: remove redundant case * fix: accomodate present files with .html in pathname * fix: typo * fix: correct path * fix: combine html/prettified url case * fix: small refactor
Related Issue
#463
Summary of Changes