Record's `container` internal attribute clashing with applicative model attributes


#1

I have the case of a model needing container attribute, but when populating, I felt on a pretty weird issue… which finally showed me that the applicative attribute clashed with the internal one.

How to deal with this? Should we consider renaming the internal attribute (e.g to _container)?


#2

@mike_aski I doubt we will change the container property to have another name anytime soon. We need to discover a better public API to replace the container before we do that.

In the near term, you should probably find another word in your domain that feel appropriate. :frowning:


#3

@mixonic

o_O ??

I am rather surprised by your answer. I did a quick patch locally, and it does only represent a really small change (exactly 2 changes, affecting the Model class & its uses):

diff --git a/packages/ember-data/lib/system/model/model.js b/packages/ember-data/lib/system/model/model.js
index bf75c02..6656509 100644
--- a/packages/ember-data/lib/system/model/model.js
+++ b/packages/ember-data/lib/system/model/model.js
@@ -375,7 +375,7 @@ var Model = Ember.Object.extend(Ember.Evented, {
   toJSON: function(options) {
     if (!JSONSerializer) { JSONSerializer = requireModule("ember-data/serializers/json_serializer")["default"]; }
 // container is for lazy transform lookups
-    var serializer = JSONSerializer.create({ container: this.container });
+    var serializer = JSONSerializer.create({ container: this._container });
     return serializer.serialize(this, options);
   },

diff --git a/packages/ember-data/lib/system/store.js b/packages/ember-data/lib/system/store.js
index c7af3f7..2511014 100644
--- a/packages/ember-data/lib/system/store.js
+++ b/packages/ember-data/lib/system/store.js
@@ -1492,7 +1492,7 @@ Store = Ember.Object.extend({
     var record = type._create({
       id: id,
       store: this,
-      container: this.container
+      _container: this.container
     });

#4

The container is used across Ember on many classes and objects. Changing these two might be trivial, but I would a) still be concerned about edge cases b) have difficulty explaining to another developer why half are container and half use _container and c) I am sure it is more challenging to change this across all of Ember, especially within a 1.x cycle (changing the location of container would break our semver).

If that patch Works For You, of course feel free to take it where you want. I doubt it is viable to push upstream though, and keep in mind that you’ve diverged your application’s codebase, making it more difficult to maintain and for new developers to understand. Fwiw, I recommend finding another path.


#5

I understand your arguments… :unamused:


#6

FYI - an issue was opened for this: emberjs/ember-data#2328 (linking so that others can follow along).