JSONAdapter: Collision with internal properties should throw warning

[Redsandro]

Ember data JSONAdapter lacks the ability to PATCH only dirty attributes. One way to work around this is to use the automatically inserted state attributes (i.e. isNew) in your serializer:

export default ApplicationSerializer.extend({
  serializeAttribute(snapshot, json, key, attributes) {
    if (snapshot.record.get('isNew') || snapshot.changedAttributes()[key]) {
      this._super(...arguments)
    }
  }
})

However, when (accidentally) using such a key in your model, no warning is given that an internally set key is overwritten, causing problems with state-related functionality.

import DS from 'ember-data'
import { computed } from '@ember/object'

export default DS.Model.extend({
  // ... abridged
  labels: DS.attr(),

  isNew: computed('labels.[]', function() {
    return this.get('labels').includes('new')
  })
})

It took me some time to figure out that my own code was to blame, indicating of course that, contrary to what my mother used to say, I'm not the smartest person in the world. I had completely forgotten this computed property and the model was not in my frame of reference when hunting for the problem.

Humans are prone to this kind of mistakes. I suggest that either the internally set parameters are prepended with an underscore to signify their status, or show a warning when internally set parameters are overwritten in the model.

Versions

─ ember-source@3.12.0 

ember-infinity@1.4.9 /tmp/ember-infinity
└── ember-cli@3.12.0 

ember-infinity@1.4.9 /tmp/ember-infinity
└── ember-data@3.12.4 

[Redsandro]

By any chance, is there a different path from which I can get() the state params that are not directly on the record?

[snewcomer]

data/packages/record-data/addon/-private/record-data.ts

Line 298 in 9a67f6a

isNew() {

@Redsandro 👋 Glad you found the issue. Which one "won" - the ember-data property or your property?

Also, luckily, this is tracked privately with _isNew already! I believe the public property isNew is meant to be overridden if you so desire.

data/packages/store/addon/-private/ts-interfaces/record-data.ts

Line 58 in 681d922

isNew?(): boolean;

[Redsandro]

this is tracked privately with _isNew already!

data/packages/record-data/addon/-private/record-data.ts

Lines 298 to 300 in 9a67f6a

	isNew() {
	return this._isNew;
	}

I can't believe I haven't found that! I should have used that. Seems like a 1 char serializer fix. I never really check out the underscored parameters when inspecting (anymore) because those change sometimes without notice. But in this case it would have been a quick fix.

Which one "won" - the ember-data property or your property?

The Ember Data property won 😅 I changed all refs in my app.

[Redsandro]

Not sure if I should close this because I understand what happened, or keep this open because everyone can step into the same trap, especially since record.get('isNew') is documented here and there on the Ember forum and Ember Igniter guides, while ('_isNew') is not.

#confusion

[sly7-7]

In the past there has been more or less the same complaint about other properties, so some tests were added: https://github.com/emberjs/data/blob/master/packages/-ember-data/tests/unit/model-test.js#L703. Here it seems like a different case, but I don't know if properties like isNew (which are defined as readOnly (

data/packages/model/addon/-private/model.js

Line 89 in 9a67f6a

isNewCP = computed('currentState', function() {

) are meant to be overwritten.
Also maybe when RecordData features are on, this problem will disappear by itself.
cc @runspired

[Redsandro]

References

• 0000-record-data.md
• [META QUEST] Request for Community Help - RecordData! #5618

[sly7-7]

Maybe we also could add some checks, like https://github.com/emberjs/data/blob/master/packages/model/addon/-private/model.js#L1399

[runspired]

A few things:

1. This property is decently safe to override as the snapshot should expose this without you needing to access it from the record.

2. Serializing non-attrs and only changed attrs and using PATCH are all relatively easy to accomplish. I'd suggested reading the adapter docs more (or reading the adapter package + MinimumAdapterInterface docs when those land in 3.16)

For non-attrs, snapshot does give you access to the record it is a snapshot for, so you can serialize any non-attributes that way via direct access.

For changed attrs snapshot gives you changedAttributes() which returns a dictionary of attributes that have changed along with their old and new values.

snapshot.isNew() returns whether the snapshot is in the new state.