media_types API is not good

[jakajancar]

The media_types API employs too much magic leading to unexpected behavior:

lookup('html');                     // 'text/html'
lookup('file.html');                // 'text/html'
lookup('path/to/file.html');        // 'text/html'

contentType('html');                // 'text/html; charset=utf-8'
contentType('file.html');           // 'text/html; charset=utf-8'
contentType('path/to/file.html');   // 'path/to/file.html' - uhh, what??

Also, the naming, while understandable for someone who knows the implementation (e.g. how the database is keyed), is a bit confusing for a newcomer:

• lookup()? Aren't they all lookups of some type or another?
• lookup() returns a (non-full?) "content type", which you pass to contentType() and it returns a "full" content type" ?

So many exports/functions:

• lookup(path)
• contentType(type)
• extension(type)
• charset(type)
• extensions
• types

A (imo) much easier to understand API would be to just expose (maybe a subset of) the database record type:

export interface MediaType {
    identifier: string,
    extensions: string[],
    charset?: string,
    compressible?: boolean
}

export function lookupByIdentifier(identifier: string): MediaType|undefined
export function lookupByExtension(path: string): MediaType|undefined

[kitsonk]

It was a port of https://github.com/jshttp/mime-types which is the defacto standard. The interfaces as they stand align well to sort of functions needed when creating a web server, as things like charset, translating both directions, etc. The API serve the minimal purpose of performing those functions, returning what is just needed and aligning to the behaviour a server would need.

While your suggestion is understandable it would be a breaking change with already built servers (as well as make it difficult for others to adapt code based on the mime-types package, as well a introduce APIs that would require people to discard stuff upon lookup in most cases.

Could there be improvements to the README to make it clearer what the APIs do?

[jakajancar]

If the API is locked down and README is all that can be fixed, then the most important thing to clarify in the docs is that while lookup() accepts extensions, filenames and paths, contentType() (and probably others) only accept extensions and filenames, but not paths, and passing in a path will return it.

[ry]

Can we add another module which implements this better API as @jakajancar suggests and leave a legacy module?

contentType('html');                // 'text/html; charset=utf-8'
contentType('file.html');           // 'text/html; charset=utf-8'
contentType('path/to/file.html');   // 'path/to/file.html' - uhh, what??

^-- I didn't realize.. That is terrible.

[kitsonk]

^-- I didn't realize.. That is terrible.

One could argue that contentType is really internal and should be exported, since it is lookup() that orchestrates the more granular APIs (and uses contentType internally), but it was exported in the original module.

[kitsonk]

Looking at oak, I realised that this API serves a useful purpose. The purpose of the API is to normalise content-types, when users take shortcuts. See how this API is used in oak:

https://github.com/oakserver/oak/blob/99dfcda9d590b73a449e1258c7c0c2e4ab790b45/response.ts#L33-L40

So I would prefer to not to change the API, it isn't broken. Clearing up documentation of its purpose, or changing the name of the API to normalizeContentType would be better. The reason it resolves "extensions" is so that a user can set the content type to a response to something like "json" but end up with a full valid media type as output. It was never designed to do paths, as that feature is lookup().

[ry]

@kitsonk lookup() takes a path but contentType() takes something like a content-type which is then passed to lookup(). I ran into this problem earlier today while editing std/http/file_server.ts

In the tests it seems like contentType() is supposed to take a path too: `

assertEquals(contentType("file.json"), "application/json; charset=utf-8");

but it's very surprising that the answer is different when you put in ./file.json instead of just file.json.

I think normalizeContentType makes sense - but I don't think it should also operate on filenames.

[kitsonk]

@ry I would be fine with that. I don't think it would break too much upstream. I think it was in the original library because people would just be lazy and pass the basename of a file as the content type, which is silly. It should be either an extension or a content type which needs to be normalized.

[crabmusket]

I'm a little late to this, but given this:

It was a port of https://github.com/jshttp/mime-types which is the defacto standard.

Wouldn't it make sense for this to live in std/node? And maybe depend on a standard-library module with a lower-level/less-magical API?

EDIT: oh, sorry - mime-types isn't actually part of node 🤦‍♂. So it wouldn't belong in std/node I assume. Ignore me!

[kitsonk]

It is now available on oakserver/media_types and https://deno.land/x/media_types/