Initial implementation of the Snapshot API #2623

wecc · 2014-12-23T23:19:11Z

This is a first stab at the new Snapshot API, serializer.serialize() now receives a Snapshot instead of a record instance.

Caching - Right now, attributes are cached eagerly (on instantiation) and relationships are cached on first access. Is this the design we want?
snapshot.belongsTo() - Returns a new snapshot (with at least an ID) or undefined. Is this what we want or should we always return a new snapshot?
snapshot.hasMany() - Always returns an array.
snapshot.get() deprecated - ~~To only use attr(), belongsTo() and hasMany() would require some work in serializers. Should that be done in the scope of this PR or separately?~~ Done in this PR.

[@sly7-7]: Just to clean issues when that awesome work is merged:
Closes #2551, Fixes #2385, Fixes #2508, Closes #1491

Fixes #2750, Closes #2739

stefanpenner · 2014-12-24T00:50:05Z

packages/ember-data/lib/system/snapshot.js

+  @private
+*/
+var Snapshot = function(record) {
+  var attributes = Object.create(null);


Ember.create to get the polyfil

fivetanley · 2014-12-24T04:19:49Z

super cool!

stefanpenner · 2014-12-24T13:25:01Z

Ah if it's an ordered set forEach is perfect

sly7-7 · 2014-12-24T13:27:52Z

I think there is a problem with the way of relationships are built. Since they are lazily cached, there may be some inconsistencies if for example a snapshot is created, its relationships are accessed, and in the meantime the record's relationships are modified. Unless I'm wrong could lead to wrong snapshot's relationships values.

wecc · 2014-12-30T21:42:02Z

packages/ember-data/lib/system/model/model.js

@@ -936,6 +937,10 @@ var Model = Ember.Object.extend(Ember.Evented, {
    this._notifyProperties(dirtyKeys);
  },

+  snapshot: function() {


Underscore this, add doc @Private

wecc · 2014-12-30T22:40:36Z

packages/ember-data/lib/system/snapshot.js

+    if (relationship && relationship.relationshipMeta.kind === 'hasMany') {
+      return this.hasMany(keyName);
+    }
+    return this.attr(keyName);


Check if keyName is attr, if not, pass to this.record.get()

igorT · 2014-12-31T19:31:23Z

snapshot.hasMany('foo', { ids: true })

rwjblue · 2015-01-04T23:07:15Z

packages/ember-data/lib/system/snapshot.js

+
+var get = Ember.get;
+
+var hasPropertyAccessors = Ember.__loader.require('ember-metal/platform')['hasPropertyAccessors'];


This is exported by Ember as Ember.platform.hasPropertyAccessors since at least 1.8.

https://github.com/emberjs/ember.js/blob/v1.8.1/packages/ember-metal/lib/main.js#L183-L186

tomdale · 2015-01-26T01:55:29Z

@knownasilya As @wecc described, the high order bit for me is that a snapshot represents the frozen state of a record at a particular moment in time. As we push everything towards being all-async all the time, you want to get a way to examine the current state of that record in the moment without triggering all sorts of side-effects, like loading relationships.

knownasilya · 2015-01-26T02:10:59Z

Thanks for the clarification @wecc and @tomdale. It sure sounds like a useful and needed API, especially in the async scenario. I've struggled numerous times trying to get state without triggering any observers/cps in complicated scenarios, and it's a battle.

sly7-7 · 2015-01-26T13:36:04Z

@tomdale @wecc From what I understand from the code, I think there are still inconsistencies in the implementation. Since the relationships are lazy and the attributes are eager, actually when you create a snapshot, and the access its data, I think problems can occur if the relationships of the underlying are changed before the snapshot's ones are accessed. Though I just may be completely wrong.

tomdale · 2015-01-26T19:45:54Z

@sly7-7 You're absolutely correct. We discussed this at length and decided that you need to pre-emptively access the snapshot's relationships if you want to freeze them at that moment in time. If we eagerly snapshot all relationships as well as attributes, we may have to create thousands of snapshots that the user never needs. Definitely open to other suggestions here, because it's going to trip people up. But I don't know how to avoid making it easy to snapshot the entire object graph otherwise.

bakura10 · 2015-02-01T15:16:36Z

I'm not sure to completely understand this feature. Can it be used to create "forked models" (à la EPF), so hat modifications are applied locally, and then saved independently (to avoid the very famous binding problem where properties are updated visually while the model is not saved yet)

tomdale · 2015-02-01T18:31:04Z

@bakura10 It is not that complex. It simply, at snapshot time, loops over all of the attributes of a model and copies their current values over into an immutable hash, giving you a "snapshot in time" of the state of that model.

thomasvanlankveld · 2015-02-11T12:22:52Z

If this does what I think it will do, it's going to make me very very happy :)

bmac · 2015-02-12T22:21:10Z

packages/ember-data/lib/system/snapshot.js

+    @method id
+    @return {String} The ID of the snapshot's record
+  */
+  id: function() {


I'm nervous because now if you do record.id it will return the id of the record. This could break serializers that are relying on this behavior.

wecc · 2015-02-13T08:26:19Z

Rebased, merge conflicts fixed.

bmac · 2015-02-13T20:19:01Z

packages/ember-data/lib/system/snapshot.js

+    ```
+
+    @method id
+    @return {String} The ID of the snapshot's record


@property id @type {String}

Initial implementation of the Snapshot API

wecc force-pushed the snapshots branch from e840003 to 7afd7cd Compare December 23, 2014 23:58

stefanpenner reviewed Dec 24, 2014
View reviewed changes

wecc force-pushed the snapshots branch 2 times, most recently from e7bf3b9 to 4e26221 Compare December 24, 2014 08:44

wecc mentioned this pull request Dec 28, 2014

[Meta issue] Ship 1.0 #2545

Closed

28 tasks

wecc reviewed Dec 30, 2014
View reviewed changes

wecc force-pushed the snapshots branch 2 times, most recently from 783db24 to 6a8b76a Compare December 30, 2014 22:35

wecc reviewed Dec 30, 2014
View reviewed changes

fivetanley mentioned this pull request Dec 31, 2014

Ember Data Meeting PR Discussion 2014/12/31 #2646

Closed

3 tasks

wecc force-pushed the snapshots branch 3 times, most recently from bb2eea4 to f4957ce Compare January 4, 2015 18:34

rwjblue reviewed Jan 4, 2015
View reviewed changes

wecc force-pushed the snapshots branch 3 times, most recently from 7e89904 to c293f30 Compare January 5, 2015 22:29

wecc force-pushed the snapshots branch from 44dcca1 to a44305c Compare February 1, 2015 19:48

wecc mentioned this pull request Feb 4, 2015

unfulfilled async hasMany relationship serializes to an empty array #2750

Closed

wecc force-pushed the snapshots branch from a44305c to e3569be Compare February 8, 2015 17:27

bmac added this to the 1.0.0-beta.15 milestone Feb 10, 2015

bmac added the Feature label Feb 10, 2015

wecc mentioned this pull request Feb 11, 2015

Model.save() errors when missing related objects #2739

Closed

bmac reviewed Feb 12, 2015
View reviewed changes

bmac added the team-review label Feb 12, 2015

wecc force-pushed the snapshots branch from e3569be to 4debd96 Compare February 13, 2015 08:25

bmac removed the team-review label Feb 13, 2015

wecc force-pushed the snapshots branch from 4debd96 to 72b3b29 Compare February 13, 2015 20:05

bmac reviewed Feb 13, 2015
View reviewed changes

wecc and others added 4 commits February 13, 2015 21:20

Initial implementation of the Snapshot API

212bd20

Belongs To Polymorphic Async Bug Failing Test

4bf6a8e

Added serializeBelongsTo with async polymorphic test for RESTSerializer

83e9cc8

Pass JSHint

b5f86f3

wecc force-pushed the snapshots branch from 72b3b29 to b5f86f3 Compare February 13, 2015 20:21

bmac added a commit that referenced this pull request Feb 13, 2015

Merge pull request #2623 from wecc/snapshots

64ccb96

Initial implementation of the Snapshot API

bmac merged commit 64ccb96 into emberjs:master Feb 13, 2015

slindberg mentioned this pull request Feb 18, 2015

Snapshot update (beta.15 compatibility) adopted-ember-addons/ember-data-model-fragments#66

Merged

runspired added 🏷️ feat This PR introduces a new feature and removed Feature labels Sep 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial implementation of the Snapshot API #2623

Initial implementation of the Snapshot API #2623

wecc commented Dec 23, 2014

stefanpenner Dec 24, 2014

fivetanley commented Dec 24, 2014

stefanpenner commented Dec 24, 2014

sly7-7 commented Dec 24, 2014

wecc Dec 30, 2014

wecc Dec 30, 2014

igorT commented Dec 31, 2014

rwjblue Jan 4, 2015

tomdale commented Jan 26, 2015

knownasilya commented Jan 26, 2015

sly7-7 commented Jan 26, 2015

tomdale commented Jan 26, 2015

bakura10 commented Feb 1, 2015

tomdale commented Feb 1, 2015

thomasvanlankveld commented Feb 11, 2015

bmac Feb 12, 2015

wecc commented Feb 13, 2015

bmac Feb 13, 2015


		var get = Ember.get;

		var hasPropertyAccessors = Ember.__loader.require('ember-metal/platform')['hasPropertyAccessors'];

Initial implementation of the Snapshot API #2623

Initial implementation of the Snapshot API #2623

Conversation

wecc commented Dec 23, 2014

stefanpenner Dec 24, 2014

Choose a reason for hiding this comment

fivetanley commented Dec 24, 2014

stefanpenner commented Dec 24, 2014

sly7-7 commented Dec 24, 2014

wecc Dec 30, 2014

Choose a reason for hiding this comment

wecc Dec 30, 2014

Choose a reason for hiding this comment

igorT commented Dec 31, 2014

rwjblue Jan 4, 2015

Choose a reason for hiding this comment

tomdale commented Jan 26, 2015

knownasilya commented Jan 26, 2015

sly7-7 commented Jan 26, 2015

tomdale commented Jan 26, 2015

bakura10 commented Feb 1, 2015

tomdale commented Feb 1, 2015

thomasvanlankveld commented Feb 11, 2015

bmac Feb 12, 2015

Choose a reason for hiding this comment

wecc commented Feb 13, 2015

bmac Feb 13, 2015

Choose a reason for hiding this comment