Rename to_json as serialized to support other output types

[GeekOnCoffee]

@steveklabnik based on comment discussion, happy for suggestions for a better name or other feedback

[steveklabnik]

?

[GeekOnCoffee]

Wow, I don't even... likely a copy/paste artifact from the one below (line 18/25), from what I can see neither one is necessary.

[chancancode]

I thought I commented, but it appears to be gone for some reason :)

Anyway - we generally should be implementing #as_json to return the appropriate Ruby object representation (a hash, array, string etc) rather than implementing #to_json

[chancancode]

And that applies to the rest of the project as well. Basically we shouldn't have any to_json in here

[GeekOnCoffee]

@chancancode replaced to_json with as_json;

A bit of context, @steveklabnik and I were talking earlier about how I'd go about supporting XML, thus the issue with defaulting the interface to a name with json... I largely just did the alias to maintain backwards compatibility, not sure if it's even necessary given the name change.

[chancancode]

I believe we used to have a more generic method called "serialized_hash", would that work for your purpose?

[steveklabnik]

@chancancode that'd be on the input side, the question is, what gets called to produce the output. The output here isn't a hash, it's the JSON/XML.

[chancancode]

If you drop the to_json call in here as I suggested in the line note, your "serialized" method is basically what "serialized_hash used to be.

Then as_json can either be defined as serialized.as_json(options) or a simple alias to serialized if you don't care about passing down the options

[chancancode]

@steveklanbnik of you defined as_json to return the Ruby hash, Rails json encoder will pick that up and do the right thing. The Object#to_json defined in Rails basically walks the object tree, convert everything along its way by calling as_json, then encode that resulting hash/array/whatever as JSON string.

So basically, if you defined as_json, to_json would Just Work TM

[steveklabnik]

Rails json encoder will pick that up and do the right thing.

Historically, this has been really bad, but I think newer Rails it's okay? I need to review.

[GeekOnCoffee]

I'm happy to help however I can, just trying to move things forward as I try and get to the point where I might attempt an XML implementation...

[steveklabnik]

I think this name is fine. This needs a rebase though, and the last build failed, though maybe it's passing now.

[chancancode]

@steveklabnik sorry I completely forgot about my comment earlier 😄

As far as I know, all currently supported versions of Rails has reasonable as_json. The main difference that you might care about (besides that it's The Right Thing™ to do 😉) is that this is basically what to_json looks like:

class Object
  def to_json(options = {})
    ::JSON.generate(self.as_json(options), quirks_mode: true)
  end

  def as_json(options = {})
    # convert this object to a hash...
  end
end

So, to_json is basically the "entry point" for JSON encoding in Rails and shouldn't be overridden. Doing so will prevent such objects from participating in the JSON encoding pipeline.

This example illustrates it well..

>> require 'active_support/json'
=> true
>> class ToJsonObject
>>   def to_json(options = {})
>>     '"zomg"' # remember, this should return valid JSON values, so the returned string should be quoted
>>   end
>> end
=> nil
>> ToJsonObject.new.to_json
=> "\"zomg\""
>> [ToJsonObject.new].to_json
=> "[{}]"
>> class AsJsonObject
>>   def as_json(options = {})
>>     'zomg'
>>   end
>> end
=> nil
>> AsJsonObject.new.to_json
=> "\"zomg\""
>> [AsJsonObject.new].to_json
=> "[\"zomg\"]"

So, unless there are very special needs here we should go with as_json project-wide, and if there are any bugs there I'd love to help looking into it and fixing it upstream 😄

Hope it helps! Sorry that I wasn't able to put a lot of time in this project, thanks for picking it up and shepherding it! ❤️ 💚 💙 💛 💜

[guilleiguaran]

I'm planning to do an adapter for Cap'n Proto so this makes sense for me 👍

[steveklabnik]

@chancancode the idea is that not everything is JSON, so having JSON in the name isn't appropriate.

[chancancode]

Yeah, the line in this pull request was fixed, so doesn't directly apply anymore. The original line that caught my attention was an addition along the lines of

active_model_serializers/lib/active_model/serializer/adapter.rb

Lines 19 to 21 in 9360a10

	def to_json(options = {})
	serializable_hash(options).to_json
	end

.

I don't have any objections about this pull request anymore, was just elaborating on my first comment and answering the question you raised 😄

[steveklabnik]

👍 ❤️

[GeekOnCoffee]

looks like the files this was based on have been moved (and changed), hard for me to tell if any changes are needed, or if this can be closed.

[kurko]

@GeekOnCoffee can you check back on this PR? I believe this can be closed now as this has already been implemented on master. Your feedback is appreciated 😁