Use MultiJson, which will pick up whatever JSON backend you're using (JSO #45

Closed
wants to merge 4 commits into from

5 participants

@seamusabshere

Use MultiJson, which will pick up whatever JSON backend you're using (JSON or Yajl or whatever).

@rkh
Official Rack repositories member

I'd rather have it ship with okjson.

@raggi
Official Rack repositories member

Just use yajl. I find it really repulsive having so much code devoted to supporting broken libs.

@rkh okjson looks pretty clean, if i ever had a desperate need to ship to non-compilation environments, that'd be useful.

@seamusabshere

@raggi I believe yajl is a C extension, which wouldn't be compatible with JRuby (?)

@raggi
Official Rack repositories member

true.

@rkh
Official Rack repositories member

AFAIK it's the only lib where you can assume decode(encode(x)) == x.

In general, I like to choose one solution when it comes to JSON, to avoid someone running into issues due to different behavior.

@raggi
Official Rack repositories member

okjson it is.

@seamusabshere

MultiJson will use OkJson (which it vendors) if neither Yajl (its first choice) nor JSON (its second choice) is available.

I think it's preferable to add multi_json as a gem dependency rather than vendoring okjson.rb in rack-contrib.

Here's the relevant code from multi_json:

REQUIREMENT_MAP = [
  ["yajl", :yajl],
  ["json", :json_gem],
  ["json/pure", :json_pure]
]

DEFAULT_ENGINE_WARNING = 'Warning: multi_json is using default ok_json engine. Suggested action: require and load an appropriate JSON library.'

# The default engine based on what you currently
# have loaded and installed. First checks to see
# if any engines are already loaded, then checks
# to see which are installed if none are loaded.
def default_engine
  return :yajl if defined?(::Yajl)
  return :json_gem if defined?(::JSON)

  REQUIREMENT_MAP.each do |(library, engine)|
    begin
      require library
      return engine
    rescue LoadError
      next
    end
  end

  Kernel.warn DEFAULT_ENGINE_WARNING
  :ok_json
end

I admit it's too bad that MultiJson warns about OkJson.

@seamusabshere

Would y'all prefer if I

  1. vendored okjson
  2. used begin / rescue LoadError like before but with the MultiJson gem
  3. ?

Thanks!

@seamusabshere

ping / should I close this?

@jamesarosen

+1. This is exactly the use-case for multi_json. Rack-Contrib shouldn't tell its clients what JSON library they should use as that's platform-dependent.

@raggi re: "I find it really repulsive having so much code devoted to supporting broken libs." If JSON were part of the standard library, then I'd agree. The "so much code" is needed to support all the environments in which Rack-Contrib could (quite reasonably) be used.

@jeremy
Official Rack repositories member

@jamesarosen btw, the json gem was pulled into stdlib in 1.9: https://github.com/ruby/ruby/tree/trunk/ext/json

@jamesarosen

@jeremy this is a fact that pleases me :)

@rkh
Official Rack repositories member

Seeing that JSON is now part of the stdlib, I don't think this is necessary anymore.

@rkh rkh closed this Dec 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment