(std/file_server): better template #3423
Conversation
zhmushan
commented
Nov 30, 2019
zhmushan
commented
| @@ -126,7 +81,7 @@ async function serveFile( | |||
| const fileInfo = await stat(filePath); | |||
| const headers = new Headers(); | |||
| headers.set("content-length", fileInfo.len.toString()); | |||
| headers.set("content-type", contentType(extname(filePath)) || "text/plain"); | |||
| headers.set("content-type", "text/plain"); | |||
There was a problem hiding this comment.
Is this the behavior we want?
There was a problem hiding this comment.
If using contentType(), sometimes the browser will download the file directly
There was a problem hiding this comment.
@zhmushan Yeah but that was my intention... I remember Python SimpleHTTPServer/http.server similarly set content types
Ref: https://github.com/python/cpython/blob/305189ecdc8322c22879a04564cad5989f937462/Lib/http/server.py#L694, https://github.com/python/cpython/blob/305189ecdc8322c22879a04564cad5989f937462/Lib/http/server.py#L854-L876
There was a problem hiding this comment.
I think we prefer to open a typescript file instead of downloading it
There was a problem hiding this comment.
@zhmushan Yeah but that was my intention... I remember Python
SimpleHTTPServer/http.serversimilarly set content types
Ref: https://github.com/python/cpython/blob/305189ecdc8322c22879a04564cad5989f937462/Lib/http/server.py#L694, https://github.com/python/cpython/blob/305189ecdc8322c22879a04564cad5989f937462/Lib/http/server.py#L854-L876
It is a pity that the mime type of typescript has not been determined yet.
Maybe we can do special processing for .ts files.
It seems that content types that start with "application/" all will be downloaded directly.
So I think it is reasonable to set it to "text/plain"
There was a problem hiding this comment.
Let's go with the behavior @zhmushan has changed for now - it's quite annoying when browsing files that they get downloaded...
std/http/file_server.ts
Outdated
| `; | ||
| } | ||
|
|
||
| // TODO: load files using template |
There was a problem hiding this comment.
What does this mean? I would opt that you remove this TODO.
There was a problem hiding this comment.
if we don’t need to render markdown/code pages, I will remove this todo
|
If you want to support custom templates Consider defining a template interface As long as the template that meets this interface can be used eg. interface Template {
// defined render
renderFile(): string
renderFolder(): string
}
class CustomerTemplate implements Template {
renderFile() {
//
}
renderFolder() {
//
}
}
function generateTemplate (tpl Template, tree): string {
//
}The interface need to be discussed May be a bit complicated, but it can be highly abstract |
std/http/template.ts
Outdated
| @@ -0,0 +1,111 @@ | |||
| import { EntryInfo } from "./file_server.ts"; | |||
There was a problem hiding this comment.
@zhmushan Please merge this file into file_server.ts - I don't think the extra module is needed.
|
This change reduced the size of the file_server bundle by half! I suppose that's because it no longer needs the content-type database?
|
