New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The many uses of Model#toJSON() #2134

Closed
ericallam opened this Issue Jan 15, 2013 · 9 comments

Comments

Projects
None yet
8 participants
@ericallam
Copy link

ericallam commented Jan 15, 2013

Currently in the docs and in the community, it seems toJSON has two uses:

  1. Turn a model's attributes into an object to be stringified before being sent to the server in sync. Overriding toJSON on a model gives the user the ability to construct a custom representation of a resource to be sent to the server. There is no other way to do this, accept for overriding sync?
  2. To clone a model's attributes into a plain object to be used in template rendering.

Once a model has overridden toJSON to get the functionality described in the first use case, the second use case is broken. For example, let's say that I have a TodoItem model, and my server expects there be a root todo property in the JSON representation of this model, I could override toJSON like so:

var TodoItem = Backbone.Model.extend({
  toJSON: function(){
    return { "todo": _.clone(this.attributes) };
  }
});

Now in my View, it no longer makes sense to use toJSON when rendering a template, so instead I search the documentation and see that the documentation for render uses toJSON, so then, starting to feel a little crazy, I head back over to the toJSON docs and find this:

Return a copy of the model's attributes for JSON stringification. This can be used for persistence, serialization, or for augmentation before being handed off to a view

I think it makes sense to use toJSON for stringification (since that's what is in the JSON.stringify spec) but it makes less sense to overload the it with "augmentation before being handed off to a view".

My suggestion would be either to alter the docs to clarify a single purpose for toJSON, or maybe even introduce a new function for the "augmentation before being handed off to a view" functionality.

@caseywebdev

This comment has been minimized.

Copy link
Collaborator

caseywebdev commented Jan 15, 2013

I agree, toJSON should be for prepping a model for serialization. I don't think we need to add a new function for view preparation, though, as that's very easy to do on a case-by-case basis. If this is the consensus we can certainly update the docs to reflect it.

@braddunbar

This comment has been minimized.

Copy link
Collaborator

braddunbar commented Jan 15, 2013

Agreed! toJSON shouldn't be doing double duty in the docs. Let's drop the examples for view rendering and the extra description.

@tbranyen

This comment has been minimized.

Copy link
Collaborator

tbranyen commented Jan 15, 2013

My recommendation has always been to never use toJSON for rendering, but instead use this.model.attributes or _.clone(this.model.attributes).

However this breaks down for collections where you want all Objects flattened, so you use toJSON there to do it. Since you never prepare collections for saving, this method works fine.

braddunbar added a commit to braddunbar/backbone that referenced this issue Jan 15, 2013

@braddunbar

This comment has been minimized.

Copy link
Collaborator

braddunbar commented Jan 15, 2013

Addressed in #2135.

@tgriesser tgriesser closed this Jan 15, 2013

tgriesser added a commit that referenced this issue Jan 15, 2013

Merge pull request #2135 from braddunbar/tojson
Fix #2134 - Clarify purpose of toJSON in documentation.
@domchristie

This comment has been minimized.

Copy link

domchristie commented Jun 7, 2013

@tbranyen is there anything wrong with passing a model (or collection) instance directly to a template? i.e. without cloning its attributes, and using model.get('attr') or model.escape('attr') in the template

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Jun 7, 2013

@domchristie -- that's often the best way to go. Usually because you've defined other highly useful methods on your model.

@domchristie

This comment has been minimized.

Copy link

domchristie commented Jun 7, 2013

@jashkenas — Thanks. Perhaps this should be the documented way to provide a template with a model (?)

@robertgorecki

This comment has been minimized.

Copy link

robertgorecki commented Jun 7, 2013

Actually I don't think that's the best way at all. Providing model can encourage using methods and complicated conditional statements but template isn't a place for this kind of logic! Keep it simple as possible and my advice will be to provide only attributes.

@braddunbar

This comment has been minimized.

Copy link
Collaborator

braddunbar commented Jun 7, 2013

Just a quick side note, if you're using _.template and call the function with your view as context you rarely need to pass anything since you can use this inside the template. It's my favorite way to go as I think it's the most elegant.

var View = Backbone.View.extend({
  template: _.template('<%- this.model.get('attr') %>'),
  render: function() {
    this.$el.html(this.template());
    return this;
  }
});
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment