fixed served directories with index.html #22
Conversation
Hi @oscarotero , and thank you very much for submitting this PR. It appears a test is failing on this branch. Make sure all tests are passing, and also add a test case that covers this case. That would be great. 👍 |
| @@ -91,7 +91,7 @@ const handleFileRequest = async (req: ServerRequest) => { | |||
|
|
|||
| const handleRouteRequest = async (req: ServerRequest): Promise<void> => { | |||
| try { | |||
| const file = await readFile(`${root}/${entryPoint}`) | |||
| const file = await readFile(`${root}${req.url}${entryPoint}`) | |||
There was a problem hiding this comment.
I think this will break SPA functionality, no?
If your root is for example /app, and you request /app/some-route/, it will look for /app/some-route/index.html which doesn't exist since it is still in /app/index.html
There was a problem hiding this comment.
Mmm, ok I didn't understand the purpose of entryPoint, because by default is index.html. Maybe we need an option to decide the router behaviour. Somethink like spa: false (so it will work as a simple static file server, but resolving paths to directories appending "index.hml" or spa: "/app.html" (so any not found request will resolve to the "app.html" file (or any other that's configured)
There was a problem hiding this comment.
Oh my, I just realized that I had not included the directory listing / static file server documentation in the readme.
Denoliver has something similar denoliver -l which disables routing and resolves paths to directories instead.
Have you tried that out? I will update the readme. Thanks for pointing it out
There was a problem hiding this comment.
Ok, that works better but does not fix the whole problem. In my opinion, if a directory contains an index.html file, it shouldn't list the files, but show this file, the same behaviour as any webserver. Can this behaviour be changed? Or provide an option for that?
There was a problem hiding this comment.
Another issue I found (not related with this one) is that BrokenPipe error appears randomly:
This is due req.respond() is an async function, so it should be prepended with an await, so the error can be catched:
try {
await req.respond({...});
catch (err) {
//ignore error
}There was a problem hiding this comment.
Ah, now I see what you mean. Sorry about that. No, that is definitely not intented behaviour. index files should be served if they exist. Do you want to work on this? It needs to take the list flag into account when determining if it should serve the index.html file or not.
Also a test for this case would be great.
Regarding the other issue, feel free to open a PR. Otherwise I will get to it soon
|
Hey @oscarotero , Just wanted to let you know that I am working on a new major version which implements the new native HTTP API that Deno released in |
Hi, I have a directory with pretty urls, so requesting for urls such
/about-me/, it should serve the file/about-me/index.html. This does no work in v2.0, so made this change to fix it.