Support default json decoder even when nil responds to :load
#1563
Conversation
…eSupport::Dependencies::Loadable mixin). Enable default json decoder to work in this case
There was a problem hiding this comment.
Wow this is a nifty find, thank you for fixing it!
There are a couple Rubocop offences to fix in order to make this pass (I've suggested a change for the first, and the second is a trailing whitespace in the spec file).
I've also requested the addition of a comment to explain what's going on, to make sure future contributors are aware of this issue.
As an alternative, we could also add a guard at the beginning of the method:
return [::JSON, :parse] unless @decode_optionsAdding this guard would mean we don't need to use the safe navigator afterwards, it will make the change more explicit and won't require the # rubocop:disable comment, so please consider this alternative (the test shouldn't need to change either!)
lib/faraday/response/json.rb
Outdated
| @@ -60,7 +60,7 @@ def process_parser_options | |||
| @decoder_options = | |||
| if @decoder_options.is_a?(Array) && @decoder_options.size >= 2 | |||
| @decoder_options.slice(0, 2) | |||
| elsif @decoder_options.respond_to?(:load) | |||
| elsif @decoder_options&.respond_to?(:load) | |||
There was a problem hiding this comment.
Rubocop complains here because nil responds to respond_to, so we need to disable it explicitly.
Could you also please add a comment stating why we're doing this, as I'm sure we'll forget in future and might decide to remove it, introducing a regression 😅
| elsif @decoder_options&.respond_to?(:load) | |
| elsif @decoder_options&.respond_to?(:load) # rubocop:disable Lint/RedundantSafeNavigation |
Description
In Rails (some versions at least), nil responds to
:load(via ActiveSupport::Dependencies::Loadable mixin). Default json decoder should still work in this case.