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

Update mime to v2.2.0 #218

Closed
wants to merge 1 commit into from
Closed

Conversation

mathiasbynens
Copy link
Contributor

No description provided.

@jfhbrook
Copy link
Owner

A bunch of people depend on the mime files capability, though. Do you know of a way to mitigate this?

@jfhbrook
Copy link
Owner

I'm trying to dig up some relevant content and figure out who I have to at

@jfhbrook
Copy link
Owner

The most recent time we punted this #212 and I think it links to most of the relevant issues.

I don't actually know how popular the mime files format is, or what it would take to replace it. Maybe we can find some people involved in the mime module we're using to comment on why they removed it?

/cc @dotnetCarpenter @mk-pmb might be relevant to your interests? no obligation to comment etc

@jfhbrook
Copy link
Owner

It should also be noted that a bunch of the tests are failing 👇

@mathiasbynens
Copy link
Contributor Author

Yeah, I just wanted to get the ball rolling on this again. I can look into fixing up the PR if there is interest.

A bunch of people depend on the mime files capability, though. Do you know of a way to mitigate this?

It’s a breaking change for sure. Bumping the major version (per semver) would prevent those depending on this functionality from suffering any breakage.

@jfhbrook
Copy link
Owner

It prevents immediate breakage but also doesn't supply a migration path, which is still pretty unfortunate.

@mk-pmb
Copy link
Contributor

mk-pmb commented Feb 19, 2018

I have an unstable branch where my migration strategy is to provide a hook for users to supply their own MIME lookup function, individual for each ecstatic instance. IIRC the old mime package had the problem of keeping a global database for itself, so all modules customizeing "their" MIME would also customize all other modules that use it.

Bonus features I'd like to see soon[TM]:

  • Have a per-ecstatic-instance default MIME type option: This string is used if the MIME function returned something false-y.
  • In case the MIME function returns something that looks like a then-able, defer serving until its results are in, with reasonable timeout. (late false-y result still means default.)

@jfhbrook
Copy link
Owner

Thenable's a good idea. Otherwise I guess we'd expect the v2 API, include a basic wrapper for the v1 API and drop cli support?

That could be a reasonable compromise.

@mk-pmb
Copy link
Contributor

mk-pmb commented Feb 19, 2018

we'd expect the v2 API

In which way "expect"? We'd use the MIME v2 API for the default lookup of course.

If users want to customize that lookup in any way, they can provide a function that looks up a MIME type by filename. If we're generous we can supply additional context about the ecstatic instance and/or current request as a 2nd argument. If they want to use the "mime" module (or any other) under the hood, its API is their problem.

@mk-pmb
Copy link
Contributor

mk-pmb commented Feb 19, 2018

Sorry for the mixup. I meant the ecstatic instance, there's no FTP here. :-)

@mk-pmb
Copy link
Contributor

mk-pmb commented Feb 19, 2018

Even better compromise: If the user MIME function yields a false-y result, that means ecstatic shall figure out on its own, using its default strategy (currently: mime v2).
This way, a user function for some few specials could be as easy as

function customMime(filename) {
  var fext = (filename.match(/\.(\w+)$/) || false)[1];
  return ({
    txt: 'text/comma-separated-values',
    ico: 'image/bitmap',
  })[fext || ''];
}

@jfhbrook
Copy link
Owner

oh I gotcha, we'd do mineLookupFunction: ((fname) => mime2.getType(fname)) or some such as a default and let people set that themselves. I like it!

I think as far as accepting thenables, I think that can be done in a separate PR.

@mathiasbynens if this proposal makes sense to you, and I'm not missing something, and you want to implement this rq (it shouldn't be too bad, the options parsing is a bit involved but easy to copy), I'd merge it.

@mk-pmb
Copy link
Contributor

mk-pmb commented Feb 20, 2018

As the default for mineLookupFunction I'd use either null (means ecstatic shall not even try to use the custom func) or a function that always returns a false-y value (e.g. () => {}), thus triggering the fallback condition. If we're going to expose our default lookup mechanism, we should so so via an explicit API, not a config default.

@jfhbrook
Copy link
Owner

without a default it would break, no? There's no behavior difference here.

@mk-pmb
Copy link
Contributor

mk-pmb commented Feb 20, 2018

Yes, behavior is the same, it's just ninja protection. Avoiding to expose a useful MIME handler via config is just meant to help people not be overly "clever" by depending on an accidential API feature.

@BigBlueHat
Copy link
Contributor

Is the key blocker here the regression of the ability to load a custom .types file (i.e. the .load() system being removed from mime 2.x)?

The mime-db project (from where mime gets its JSON type files) contains some code for parsing (via regex) and writing the JSON format that mime-db and mime use:
https://github.com/jshttp/mime-db/blob/master/scripts/fetch-apache.js

There are two conceivable approaches (so far 😉):

  1. add this sort of code and continue to handle .types file references (converting the them into the JSON mime needs at runtime)
  2. ship a script which converts .types to these .json files and then passing those .json file references in via the command line

My preference would be for option 1 as the command line "signature" and expectations would be the same, but I can understand the motivations behind option 2.

Thoughts?

@jfhbrook
Copy link
Owner

jfhbrook commented Jun 19, 2018

There's also

  1. Lose the built in cli capability of parsing .types files, but add the option of supplying a custom mime lookup function (programmatically) which may or may not leverage mime@1

which was suggested by mk-pmb, who iirc is the most vocal person around this sort of functionality, so it works for me.

@guybedford
Copy link

This would be good to support WASM, as it still doesn't serve in Firefox with this server as far as I can tell.

@jfhbrook
Copy link
Owner

jfhbrook commented Apr 5, 2019

Closing in favor of #240

@jfhbrook jfhbrook closed this Apr 5, 2019
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.

None yet

5 participants