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

Call model.toJSON always when sync model #4179

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AriasBros
Copy link

@AriasBros AriasBros commented Dec 28, 2017

When you save your model with patch option to true, toJSON isn't called. This behavior goes against the issue #1019 where it tells that:

toJSON is already the complement of parse

With this PR the default behavior is the same as the current one, but it gives us the opportunity to manipulate the attributes before send them to the persistence layer, either using a POST, PATCH or PUT method.

@ogonkov
Copy link
Contributor

ogonkov commented Apr 7, 2019

If you have to manipulate attributes before sending, why not pass already manipulated object in attrs?

@AriasBros
Copy link
Author

AriasBros commented Apr 18, 2019

If you have to manipulate attributes before sending, why not pass already manipulated object in attrs?

Because the object attrs should store data in the format that is consumed by your application, not in the format of your persistence layer.

And, if toJSON is the complement of parse, it should be called always before send data to the persistence layer, in the same way that parse is always called when data is received from the persistence layer.

For example, a model with the next parse implementation:

parse(response, options) {
    response.rating = response.rating * 2;
    return response;
}

In the persistence layer, rating is stored as an integer from 0 to 5. But in our app we use rating as a number from 0 to 10.

Now, we want update the rating in our model:

model.save({ rating: 7 }, { patch: true });

Currently, with this line, our model will be updated to rating 7. And the persistence layer also will receive a 7 as rating, which is wrong. This is failing also in the premise that toJSON is the complement of parse, because it is not being called.

You are suggesting to do:

model.save({ rating: 7 / 2 }, { patch: true });

Which is also wrong, because yes, your persistence layer will receive a rating of 3.5, that is the expected value. But your model will have a rating of 3.5 too, and that value is not what you want in your application.

With this PR, in the same way that we have a parse method in our model, we will have the toJSON method too:

toJSON(options) {
    let attributes = Backbone.Model.prototype.toJSON.apply(this, arguments);
    attributes.rating = attributes.rating / 2;

    return attributes;
}

@ogonkov
Copy link
Contributor

ogonkov commented Apr 27, 2019

Very neat. Now it make a sense for me. Thank you for a great explanation.

@ogonkov
Copy link
Contributor

ogonkov commented Jan 10, 2022

It is great PR, but i think we should add some flag, to switch for new behaviour, because it is pretty much could break someones apps

Backbone.useToJSON = false;

@jgonggrijp jgonggrijp self-assigned this Jul 27, 2023
@jgonggrijp
Copy link
Collaborator

@ogonkov I agree with you, but I think I want this behavior to be the default in a future Backbone 2.0. How about we name the flag Backbone.future instead of Backbone.useToJSON, and then potentially also use that flag for other behaviors that we want to change in the future?

@ogonkov
Copy link
Contributor

ogonkov commented Jul 27, 2023

One flag for few features? @jgonggrijp

@jgonggrijp
Copy link
Collaborator

Well, changed features. That we intend to become standard.

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

Successfully merging this pull request may close these issues.

3 participants