Skip to content
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

Remove BakedFile#mime_type #17

Closed
straight-shoota opened this issue Feb 19, 2018 · 5 comments
Closed

Remove BakedFile#mime_type #17

straight-shoota opened this issue Feb 19, 2018 · 5 comments

Comments

@straight-shoota
Copy link
Collaborator

BakedFile doesn't need property mime_type. The mime type of each file can be queried at runtime. In fact, storing the mime type at compile time could lead to some unexpected behaviour when the mime type registry of the build system differs from the execution environment. I can't see any valid use case for having this property.

@lipanski
Copy link

I'm bundling assets inside a web app binary - it helps with setting the Content-Type header.

@straight-shoota
Copy link
Collaborator Author

You can get the same values (or even more accurate ones) at runtime. There's no need to have this in the baked file. Regular File instances don't have expose any accessor for retrieving a mime type either.

Take a look at the current code to see how you can implement it in user code:

io << " mime_type: " << (mime_type(path) || `file -b --mime-type #{path}`.strip).dump << ",\n"

@rwojsznis
Copy link

rwojsznis commented Mar 23, 2018

Hello @straight-shoota - and what about adding mtime instead? I'm trying to implement sane Etag headers support for serving baked files from Kemal and I was about to prepare PR that adds this handy property, but maybe there is a better way? 🤔

update: ok I guess I can just calculate base64digest from the file content itself and just roll with that, should do the job I suppose /shrug

@straight-shoota
Copy link
Collaborator Author

Yeah, I have already suggested that in #16:

In theory, File and BakedFile should be pretty much interchangeable so that applications can easily switch (or combine) loading files from real and virtual file systems. [...]

We could discuss adding for example modification or creation date and hopefully find a way to make this accessible in a way similar to File. A modification date would probably help setting up a custom etag generator.

But that's not related to this issue.

@straight-shoota
Copy link
Collaborator Author

About the issue with mime types: I have a PR pending to add a MIME registry to the crystal (crystal-lang/crystal#5765). When this gets merged and released we can remove BakedFile#mime_type as it can easily be replaced with MIME.fetch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants