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

defaultValue should make models dirty #2566

Closed
courthead opened this issue Dec 9, 2014 · 8 comments
Closed

defaultValue should make models dirty #2566

courthead opened this issue Dec 9, 2014 · 8 comments

Comments

@courthead
Copy link

In the JSBin below, I've got an App.Book model with a default value for the title attribute, and a book instance retrieved from the server that has no value for its title attribute:

http://emberjs.jsbin.com/xaniheguto/2/edit?html,js,output

The book's title is automatically set via the defaultValue option. However, the model isn't marked dirty and model.rollback() doesn't set the title to undefined. I might be alone here, but I would expect any model that's changed in memory to be marked dirty so I know whether or not it differs from the state on the server, and the fact that defaultValue is an exception to that behavior is a surprise.

One use-case might be if you're using Firebase, Dropbox Datastore API, etc as a back-end. You can't write server code to validate your data, so you have to do it on the front-end. In this particular case, you'd have no practical way of knowing that the book's title on the server is actually null.

@knownasilya
Copy link
Contributor

I agree as well, although maybe there should be a way to override the behavior. For example, if you have a title attribute, and you set defaultValue: 'Unknown', this is more of a UX thing and doesn't represent a change in the model's data, just how that data is shown.

@BryanCrotaz
Copy link
Contributor

I disagree - if you want a default value in the UI, use a controller. For my money, defaultValue should only be used when creating new records. It shouldn't override what we get from the server.

@bmac
Copy link
Member

bmac commented Oct 13, 2015

It is surprising to me that defaultValue is used when the record is loaded from the server and not created locally. Unfortunately, I don't think we can change this behavior without breaking semver so its going to have to stay.

I'm going to close this issue. Feel free to re-open if you disagree.

@bmac bmac closed this as completed Oct 13, 2015
@knownasilya
Copy link
Contributor

What about making the change for 3.0?

@courthead
Copy link
Author

+1 for changing this in 3.0

@runspired
Copy link
Contributor

To restate the original request here:

defaultValue on a newly created record should cause the newly created record to be dirty. I'm not sure if I agree with this, but I do understand how it could be useful.

My concern is that the use cases for "create a record with default values and save it" sounds suspiciously like "API actions". For instance, likes on a like button, in which the record itself is barely cared about.

Now, it may be that an "API action" should actually be added as a relationship to something else, in which case this was valid, but more often than not I see this in the "discardable local record" situation. And, to go with our "like" example, were this a like there would be a relationship. Ergo, I don't see this feature as useful for anything other than "disregarded API actions", for which the following pattern is better, using the example of analytics gathering.

this.get('store').adapterFor('analytics').triggerAction('page-load', data);

cc @wecc

@wecc
Copy link
Contributor

wecc commented Oct 22, 2016

Dumping this here after discussion with @runspired, @stefanpenner and @tchak:

The current defaultValue implementation is not really "default – as in a UI concern. That's why they won't mark records as dirty or get serialized.

It's possible that the current defaultValue implementation should be removed completely for 3.0 or extracted into an addon since it's use is very limited (and name confusing). Default values should really be set in init() - which is not possible today due to Attempted to handle event 'didSetProperty' on <...> while in state root.empty, see #4509.

This definitely needs to go through the RFC process 📝

@runspired
Copy link
Contributor

We do serialize defaultValue at this point for snapshots (personally think this is crazy ;P ).

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

No branches or pull requests

8 participants