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

mime-types should not forcibly require json #85

Closed
postmodern opened this issue Dec 5, 2014 · 5 comments · Fixed by #111
Closed

mime-types should not forcibly require json #85

postmodern opened this issue Dec 5, 2014 · 5 comments · Fixed by #111
Milestone

Comments

@postmodern
Copy link
Contributor

@postmodern postmodern commented Dec 5, 2014

mime/type.rb requires 'json' but the mime-types gem does not dependent on any json gem. mime/types.rb seems to only require json for the to_json method. This could be moved into a separate file, or maybe only define #to_json if defined?(JSON). mime/types/loader.rb uses JSON to load data/mime-types.json, but perhaps this could be converted to YAML to avoid the json dependency?

@halostatue

This comment has been minimized.

Copy link
Member

@halostatue halostatue commented Dec 6, 2014

It is correct that the mime-types gem does not depend on any JSON gem because json.rb is part of the Ruby standard library in Ruby 1.9 and later (see #55). I also did some benchmarking of JSON against YAML against my hand-rolled Regex parser (from v1) as I was preparing to move to v2, and the JSON version was consistently faster than the YAML version (and both stomped all over my hand-rolled parser, which I was dropping anyway because it didn’t provide enough information).

The json included with the Ruby standard library works, but json-pure does not. So…I’m not going to change the default format (and it’s going to be a bit more important moving forward to 3.0 because of the competing imperatives I have with #67 and #83).

What I can probably do is remove the require 'json' from mime/type.rb (meaning that MIME::Type#to_json is defined anyway, but will blow up if you do not have a JSON library loaded), and make it only used in mime/types/loader.rb if JSON is not otherwise loaded.

@postmodern

This comment has been minimized.

Copy link
Contributor Author

@postmodern postmodern commented Dec 6, 2014

@halostatue ah ha, I believe it's because bundler is excluding json for some reason.

There's another option which would be the much efficient, but not as elegant. Instead of generating a JSON document, generate a Ruby file that defines a giant literal Array of content types.

@halostatue

This comment has been minimized.

Copy link
Member

@halostatue halostatue commented Dec 6, 2014

That’s certainly a possibility, and worth considering for 3.0—it may help with the memory use and provide somewhat smarter/lighter data loading (I’d love to be able to get lazy loading really granular, because it would do more to help #83 than anything else).

I’m still going to be generating the JSON in some form of 3.0 because I’ve ported the core functionality to Io and want to do the same for a few other languages (as a learning exercise), plus someone ported mime-types (v1) to Python a few years ago, and they recently got a issue requesting a bump to the v2 data, so I think that the software-readable data that I’m providing is of value, and JSON is a fairly clean (and mostly safe) way to do that in a way that YAML implementations are less likely to provide.

@halostatue

This comment has been minimized.

Copy link
Member

@halostatue halostatue commented Dec 6, 2014

I will certainly benchmark the startup time of an array vs. the JSON. I suspect that it will be faster than JSON, and if that’s the case, it’s worth pursuing all on its own.

@halostatue

This comment has been minimized.

Copy link
Member

@halostatue halostatue commented Nov 20, 2015

The default registry store for mime-types 3.0 is the columnar store, required with require 'mime/types' or with require 'mime/types/columnar'. The JSON store will be used if you require 'mime/types/full' or do an explicit MIME::Types::Loader.new.load(columnar: false).

I’m not pursuing the idea of a generated array because of previously noted memory usage issues.

@halostatue halostatue closed this Nov 20, 2015
halostatue added a commit that referenced this issue Nov 21, 2015
- Require Ruby 2.0 or later. Resolves #97.
- Remove deprecated methods.
- Update known registries when a MIME type extension changes. Resolves #84.
- Relicensed mime-types 3.0 as MIT only. Resolves #95.
- Extracted data from this gem to mime-types-data; removed deprecated data.
- Rewrote tests to better understand what is being tested—some of the tests
  were almost ten years old and didn’t make a lot of sense with this version. I
  have switched to minitest/spec with assertions.
- Columnar data is now the default registry store. Because JSON is not required
  by default, this change resolves #85.
- MIME::Types containers are now implemented with Set instead of Array to
  prevent data duplication. Resolves #79.
halostatue added a commit that referenced this issue Nov 21, 2015
- Require Ruby 2.0 or later. Resolves #97.
- Remove deprecated methods.
- Update known registries when a MIME type extension changes. Resolves #84.
- Relicensed mime-types 3.0 as MIT only. Resolves #95.
- Extracted data from this gem to mime-types-data; removed deprecated data.
- Rewrote tests to better understand what is being tested—some of the tests
  were almost ten years old and didn’t make a lot of sense with this version. I
  have switched to minitest/spec with assertions.
- Columnar data is now the default registry store. Because JSON is not required
  by default, this change resolves #85.
- MIME::Types containers are now implemented with Set instead of Array to
  prevent data duplication. Resolves #79.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.