Data: isDirty based on strict comparison

[Herriau]

JSFiddle: http://jsfiddle.net/U6Xc3/

It seems like to determine whether a record is dirty Ember Data uses strict comparison between the original (last committed) value and the new value. While this may arguably make sense at first, it ends up posing some challenges.

Indeed, as soon as you start playing with 2-way binding, what used to be an integer or a date may become a string, and even though the intrinsic value has not really changed (e.g. 123 probably has the same meaning as "123") Ember Data will consider that value dirty.

This becomes even more challenging as you start playing with more advanced form components that are bound to non-primitive values such as arrays (e.g. ([2,3,5] === [2,3,5]) === false). The strict comparison operator determines whether the two references point to the same object but it certainly isn't of any help when it comes to comparing the actual values conveyed by those objects.

The faulty piece of code in Ember Data:

var didSetProperty = function(record, context) {
  if (context.value === context.originalValue) {
    delete record._attributes[context.name];
    record.send('propertyWasReset', context.name);
  } else if (context.value !== context.oldValue) {
    record.send('becomeDirty');
  }

  record.updateRecordArraysLater();
};

At the very least you should consider using a coercive comparison (==), or an even more tolerant comparison such as a.valueOf() == b.valueOf() when possible. Also, if the operands implement a comparison method with a conventional name such as equals() or isEqual() it could be used instead.

I don't see how I can properly override this default behavior beside overriding all the substates in DS.RootState with a more tolerant didSetProperty, which is far from being ideal. Another solution that I considered is to specialize some Ember components (e.g. having a text field for dates, etc...) that would cast the value to one of the primitive types, but that does not really apply to all cases, this is a bit of a pain and it certainly deviates from the way things should be elegantly written using Ember.

[lukemelia]

An extension point allowing a model (or maybe a serializer?) to determine whether a field should be considered changed would be a nice touch. Identifying some use cases would be helpful in making sure we come up with the right API.

[andycrum]

I just ran into this issue as well – the same use case as the example above (I have a number attribute that is getting marked as dirty as soon as you blur out of the textfield, even if nothing has changed). This also happens for null vs. "".

[mitchlloyd]

I recently ran into this problem on a project that @rockwood and I are working on. @rockwood ended up solving the problem with a custom input component. Here is the use case:

A user enters a numeric score into a text field. When the value changes, the model should become dirty so that we can give feedback to the user and save the value to the server. However if the user merely clicks on fields or tabs through fields, the model is marked dirty.

For our use case, it would be great if setting "123" on a 'number' attr with a value of 123 did not mark a model dirty. It would also help if setting '' for a number field that is currently null didn't mark a model dirty.

[rwjblue]

We could likely either use Ember.isEqual or Ember.compare to handle this.

Thoughts? If agreeable I could work up a PR...

[decasia]

I second @mitchlloyd's use case. A closely related one is that, if you have a blank text field bound to a recently initialized string field with no default value, it defaults to null, and then the model can be marked dirty just by clicking through the field and silently setting it to "". Do people think this feature should handle that case or is it better to just have all strings default to "" to avoid the issue?

I completely agree with @Herriau that we shouldn't need to build out an entire new set of input components that do typecasting -- I'd rather see an option on the model attribute definition, something like:

note: DS.attr('string', {markDirtyWith: "strict"})
// possible options like (strict | loose | isEqual | castToString) 

I'm not sure what the performance implications would be of complicating the didSetProperty method, though.

On second thought, is there support for stricter typecasting in Ember Data setters? Such that we could do something like this and have this problem largely vanish?

quantity: DS.attr('number', { castOnSet: 'Number' })
// pass in a default or custom constructor function

At any rate, it seems unintuitive to me that it's possible to set an Ember Data number attribute to a string, now that I think about it. It looks to me like the typecasting issue has been discussed before and gone nowhere, unfortunately, but I do think it seems like something that could usefully be thought about in conjunction with the isDirty comparison question.

[Herriau]

@decasia Typecasting could be part of the solution, and in fact I have augmented my model base class to do just that. But this only solves the problem when you have primitive values such as strings and integer. As soon as you're dealing with complex types (i.e. objects), the strict comparison will fail, e.g. (new Date('12-12-2007') == new Date('12-12-2007'))

[decasia]

@Herriau I'd love to see a gist of your model base class, if you don't mind sharing your solution to the typecasting problem with the community. I do take your point about it only being a solution to the primitive comparison case, though (that's where Ember.compare could come in).

I guess my point was just that it seems logical to keep the typecasting problem in mind in this thread, since they seem like closely related issues and it's unfortunate that AFAIK the typecasting issue is currently not even being discussed.

[igorT]

I think this should be solved by keeping around the transformed attributes. It is related to #1854

[igorT]

Another solution would be to serialize the value before comparing it. This would nicely solve the problem of comparing more complex objects.

[mjwach]

A closely related one is that, if you have a blank text field bound to a recently initialized string field with no default value, it defaults to null, and then the model can be marked dirty just by clicking through the field and silently setting it to "". Do people think this feature should handle that case or is it better to just have all strings default to "" to avoid the issue?

Several months later, I've just discovered this thread after having wrestled with the version of the problem described in the above quote off and on for quite some time. In my case, defaulting to "" would have potential to cause trouble. But simply letting me define my own comparison function for use by isDirty, or letting me choose a pre-made one that equates null/undefined with the empty string, would be perfect. My server is sometimes sending serialized models that have missing fields instead of empty-string values, and although I might eventually like to change that, for now I'd just rather be flexible. But Ember disagrees. :(

After trying pretty hard to work around isDirty's quirks, I have given up on it and solved my instance of this problem by abandoning isDirty altogether and using instead my own isReallyDirty(), which compares the current field values with a set of "latest clean values" that it saves off in response to certain DS.Model events (didCreate, didLoad, maybe didUpdate). This has the added benefit of also solving the problem of the isDirty comparison's inability to look inside field values that happen to be compound objects (which can occur when you use a custom transform, as I do in one case). But I did have to write a substantial amount of code to get all that working the way I wanted, which wasn't so nice....

Since this conversation doesn't appear to have been resolved, and the bug/feature/whatever remains in Ember (as of a pretty recent beta of 1.8), I guess I will give up again on searching for other solutions and continue to expand the sophistication of my isReallyDirty() hack.

[mitchlloyd]

@mjwach I agree that isDirty is in a pretty rough spot.

I modified someone's JSBin to show two bugs:

1. When you change a relationship the model isn't dirty
2. If you just click in the text field and click away the the model becomes dirty

http://jsbin.com/jokoxegilonu/6/edit

In this other JSBin I work around the issues by

1. Using setUnknownProperty to determine when something is dirty.
2. Modifying the text input component to not update when the value hasn't really changed.

http://jsbin.com/kevomi/2/edit

These are the work arounds I've been using for now.

[superlou]

I think I'm hitting this issue when I'm trying to set a DS.attr('date'). Would be interested in a baked in solution.

[oriSomething]

is there is a new about that? isDirty is a real pain with arrays

[jpaas]

The ability to manage embedded objects/arrays and track whether they or dependent hasMany are dirty is killing me. Any solutions people have worked out in the past year are all broken on ember-data 1.19. Has anyone found anything that works on 1.19?

oh and i have this strict comparison problem with my simple number attributes too.

[andrewbranch]

+1, problem is the same with hasDirtyAttributes. I think I agree with @igorT about comparing serialized values, because that would make the changed attributes canonically represent whether or not they are out of sync with your database (or whatever persistence layer).

[fleur]

+1

Wouldn't comparing serialized values mean there would be a new restriction that serializers would always have to serialize the same way? I know you can't rely on JSON.stringify() to stringify in the same order all the time, and I would imagine that's a common function to use for serialization. That said, this is killing me too. Help.

[henrymazza]

+1

[gustavocabral]

+1

[blimmer]

For me, null and undefined dirtying the record is the biggest problem. I use this workaround in an intiailizer to work for my needs. You can customize the checkIsActuallyDirty function to meet your needs until there's a more supported solution.

[jameshoward]

The workaround to use checkIsActuallyDirty won't update when legitimately changing a value once you've made a change that causes hasDirtyAttributes to change. The problem is that changing the number to a string (e.g. 10 to "10") causes the computed isActuallyDirty to be recalculated and it (correctly) returns false. Trouble is, if you then modify the value again then hasDirtyAttributes remains true and the computed property does not get recalculated and remains false even though you have now made a change.

[tythewebguy]

I'm running into the same problem--numbers are getting converted to strings and making things dirty. It would be nice to override the dirty checking at the model level when it makes sense.

I like the suggestion @decasia had:

note: DS.attr('string', {markDirtyWith: "strict"})
// possible options like (strict | loose | isEqual | castToString) 

It would also be nice to pass the values to a function to make the comparison. Maybe in the example above, if we use a string naming a primitive type it would use type coercion for the comparison, and without strings it would use a computed property--for comparing more complex data types and objects?

Regardless, it looks like many of us are running into this problem.

[songoo]

I hit the same problem with "count" (DS.attr('number')) attribute, passed to ember input. Model got dirtied at moment of focus out even when I did not change a thing.

Only easy solution I could think of was computed property with setter, which i used to input instead, so only number will be set and not string ..

count: DS.attr('number'),
countDecimal: Ember.computed('count', {
        get(key) {
            return this.get("count");
        },
        set(key, value) {
            let number = parseFloat(value);
            this.set("count", number);
            return value;
        }
    })

{{input value=task.countDecimal ...

Did not expected this kind of trouble, same with detecting relation changes, if they just write in documentation, relationDirtiness is Application layer, use own additional isRelationDirty method based on relation attribute observers, I could spare few days of googlin :(

[XaserAcheron]

As this seems to be the closest issue for this: my eyes practically fell out of their sockets today when I discovered during a debugging session that a model's DS.attr('number') field got set to a string. If ember-data doesn't do any type casting or checking for typed DS.attr's, then what the devil are they even for? o_O

This doesn't solve the object comparison case (as mentioned a while back), but the solution for numbers ought be different. It's liable to be a breaking change for some folks, of course. x:

[kdomagal]

Issue from 2013 and still not fixed...

[cincodenada]

I ran into this today, and in digging around it was suggested in #5648 that the new RecordData interface would allow customizing the comparison used to determine hasDirtyAttributes, which is exactly what I want. ember-m3 and ember-data-model-fragments both seem way overkill for my usecase, which is just to make it so appending to an array field marks that field as dirty.

But I looked into RecordData's source (since it's not publicly documented) and didn't see a straightforward way to accomplish this - hasChangedAttributes seems to just depend on property changes being set in __attributes, which appending to an array won't do. RecordData's setDirtyAttribute(), which populates the __attributes map, looks like it's only called once internalModel determines that an attribute has changed, which means if internalModel doesn't think it's changed then RecordData doesn't have any way to mark it as dirty.

Am I missing something here, or did RecordData not end up exposing the surface needed to implement a custom comparison function for dirty checking?