Skip to content
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

lookup JSONSerializer instance through store instead of manual instan… #3211

Merged
merged 1 commit into from
Jun 5, 2015

Conversation

fivetanley
Copy link
Member

…tiation

@fivetanley fivetanley force-pushed the fix-to-json-on-models branch from e81023d to 2a90daa Compare June 5, 2015 03:10
@fivetanley fivetanley force-pushed the fix-to-json-on-models branch from 2a90daa to 8106107 Compare June 5, 2015 03:12
fivetanley added a commit that referenced this pull request Jun 5, 2015
lookup JSONSerializer instance through store instead of manual instan…
@fivetanley fivetanley merged commit 54eaf14 into master Jun 5, 2015
@fivetanley fivetanley deleted the fix-to-json-on-models branch June 5, 2015 03:15
@@ -375,7 +374,7 @@ var Model = Ember.Object.extend(Ember.Evented, {
*/
toJSON: function(options) {
// container is for lazy transform lookups
var serializer = JSONSerializer.create({ container: this.container });
var serializer = this.store.serializerFor('-default');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fivetanley Good change! Would it be possible to do something like store.serializerFor(this.constructor.typeKey) instead? To get the record-specific serializer when one is available.

We've had this work-around for a long time (code not aligned with recent ED, but it shows the main idea):

toJSON: function(options) {
  var serializer = this.store.serializerFor(this.constructor.typeKey);
  return serializer.serialize(this, options);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, if you want the record specific serializer, you record.serialize(). toJSON is using the json serializer for debugging and similar purposes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, thanks! My workaround was old, I see now that the issue I worked around has been solved for ~2 years 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants