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

feat: add as_content_type() and make from_path() public #134

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

Cre3per
Copy link
Contributor

@Cre3per Cre3per commented Feb 19, 2023

Closes #133

The mappings in as_content_type() are a suggestion based on from_content_type() and web searches.

Potential issues

from_file_extension() is case-sensitive to file extensions while from_path() is case-insensitive.
If the crate user has non-lower-case extensions, they can transform them once to avoid every library function from having to do the transformation.

Additional changes

Changed return type of as_ts_extension() to 'static. If this change is wrong, I'll also need to remove the 'static from as_content_type().

@CLAassistant
Copy link

CLAassistant commented Feb 19, 2023

CLA assistant check
All committers have signed the CLA.

@petamoriken
Copy link

petamoriken commented Feb 28, 2023

The mimemap source code in the h2o server may be helpful.

It may be a good idea to add the following MIME in as_content_type.

  • geojson as application/geo+json
  • gltf as model/gltf+json
  • jsonld as application/ld+json

And how about also supporting the above MIME in from_content_type, and treating anything that ends with +json as JSON?

FYI:
https://github.com/h2o/h2o/blob/master/lib/handler/mimemap/defaults.c.h
https://github.com/h2o/h2o/blob/e041bac7f7556415a201bc664d16412c5f6c62de/lib/handler/mimemap.c#L428-L435

@Cre3per
Copy link
Contributor Author

Cre3per commented Mar 1, 2023

@petamoriken deno_ast provides functionality for deno itself, which means MediaType only needs to support formats of scripts, configs, and so on. MediaType isn't intended to be a general purpose type. geojson, gltf, and jsonld would be unused, because deno doesn't use these formats.

I had a quick look through media types ending in +json and didn't see any that a web server would set when serving files used by deno. If you know of a file where such a media type is correct, it might be worth opening a separate issue.

@petamoriken
Copy link

petamoriken commented Mar 1, 2023

@Cre3per I suggested this because I thought it would be useful for displaying JSON in LSP and handling JSON in Import Assetions when developing a web application that uses geojson, gltf, and jsonld. I may be misunderstanding, and as you say, it would be better to address this in a separate issue.

src/media_type.rs Outdated Show resolved Hide resolved
src/media_type.rs Outdated Show resolved Hide resolved
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @Cre3per!

@dsherret dsherret changed the title feat: add as_content_type() and from_file_extension() #133 feat: add as_content_type() and make from_path() public Mar 9, 2023
@dsherret dsherret merged commit 99cea56 into denoland:main Mar 9, 2023
@Cre3per Cre3per deleted the extension-and-content-type-mapping branch March 13, 2023 12:42
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

Successfully merging this pull request may close these issues.

Map file extension to content type
4 participants