search

schovi / baked_file_system

Virtual File System for Crystal language. Embedding your assets into final binary.
MIT License
177 stars 18 forks source link

Remove `BakedFile#mime_type` #17

Closed straight-shoota closed 5 years ago

straight-shoota commented 6 years ago

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 commented 6 years ago

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

straight-shoota commented 6 years ago

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:

https://github.com/schovi/baked_file_system/blob/a23e823baf95f6b5a8f1d2b195df6a8dd7bff580/src/loader/loader.cr#L30

rwojsznis commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.