ohler55 / oj

Optimized JSON
http://www.ohler.com/oj
MIT License
3.14k stars 251 forks source link

`v3.15.1` causes a warning to be output when calling `Oj.mimic_JSON` #891

Closed magni- closed 10 months ago

magni- commented 1 year ago

The warning was added here: https://github.com/ohler55/oj/commit/6367db7827a2373e621596acbacea28f28ee7594

And gets triggered by this line: https://github.com/ohler55/oj/blob/v3.15.1/lib/oj/mimic.rb#L99

My assumption is that the warning is only meant for external code requiring oj/json?

ohler55 commented 1 year ago

It should not get triggered. Can you provide an example of how it was triggered. My tests must not be complete enough.

magni- commented 1 year ago

Our Rails app's Gemfile includes both oj and oj_mimic_json. We also have an indirect dependency on json (the latest version, v2.6.3).

When first launching the app, the warning gets printed out. I bundle open oj'ed to add puts all over which confirmed that it's this line https://github.com/ohler55/oj/blob/v3.15.1/lib/oj/mimic.rb#L99 that causes the warning to be printed out.

I inspected JSON::PRETTY_STATE_PROTOTYPE: it's the constant defined by json rather than the one defined by oj/json (https://github.com/flori/json/blob/v2.6.3/lib/json/common.rb#L76)

I confirmed this by changing that line locally to

const_set :PRETTY_STATE_PROTOTYPE, {hello: :bug}

and then adding this to oj/json.rb:

 if defined?(JSON::PRETTY_STATE_PROTOTYPE)
+  puts JSON::PRETTY_STATE_PROTOTYPE.inspect
   warn "WARNING: oj/json is a compatability shim used by Oj. Requiring the file explicitly is no recommended."
 end
> spring stop
Spring stopped
> bundle exec spring rails r "puts :ok"
{:hello=>:bug}
WARNING: oj/json is a compatability shim used by Oj. Requiring the file explicitly is no recommended.
Running via Spring preloader in process 51676
ok
ohler55 commented 1 year ago

The issue is requiring the JSON gem first. If that is done there should not be a reason to include oj/json.rb. At least that was my thinking. The issue being fixed was including the JSON gem and the oj/json caused an infinite loop. Possibly the fix is to remove or do an extra check in oj_mimic_json. Worse case I'll have the warning silenced but I'd like to explore other options first.

ohler55 commented 1 year ago

Please try the latest v3.16.0.

ohler55 commented 10 months ago

No response, closing.

magni- commented 10 months ago

Sorry, I missed the notification—it is indeed fixed, thank you!