grape-roar icon indicating copy to clipboard operation
grape-roar copied to clipboard

Published 20 hours ago • verified publisher icon ruby-grape

Drop MultiJson dependency

Open yuki24 opened this issue 6 years ago • 7 comments

It is no longer encouraged to use the multi_json gem due to this reason: https://github.com/intridea/multi_json/pull/113#issuecomment-17668823

cc @dblock

yuki24 avatar Oct 27 '17 17:10 yuki24

Can you add a CHANGELOG entry, please? 😸

mach-kernel avatar Oct 27 '17 17:10 mach-kernel

This should be good to merge once the CI is green.

yuki24 avatar Oct 27 '17 18:10 yuki24

roar depends on multi_json via representable (for prior context). I remembered now.

mach-kernel avatar Oct 27 '17 19:10 mach-kernel

https://github.com/ruby-grape/grape-active_model_serializers/blob/master/lib/grape-active_model_serializers/error_formatter.rb#L19 might be helpful, although I am not in love with the solution, maybe define Grape::JSON if it's not already defined?

dblock avatar Oct 27 '17 19:10 dblock

So why do we need to have a check against #to_json? It doesn't look like it is the same method that comes from require 'json', but I feel like there shouldn't be any case where the object doesn't respond to #to_json.

yuki24 avatar Oct 27 '17 20:10 yuki24

@mach-kernel it doesn't look like representable depends on the multi_json gem today, it used to until 2.2.3.

yuki24 avatar Oct 27 '17 20:10 yuki24

Representable actually requires multi_json for JSON support today, see https://github.com/trailblazer/representable/blob/b4439a5e025106af282f8dbc590418a108640159/lib/representable/json.rb#L4. So a PR should go upstream.

dblock avatar Oct 31 '17 20:10 dblock