[BUGFIX beta] Avoid creating enumerable properties on all objects created by DI.

[rwjblue]

Prior to these changes, every object created via owner.factoryFor(...).create() was populated with NAME_KEY, _debugContainerKey, and OWNER.

After these changes:

• For Ember.Object's, only actual injections are included in the create arguments.
• For non-Ember.Object's, only OWNER is included in the create arguments.

OWNER is still provided for non-Ember.Object's in order to support various classes that are used in the rendering layer that expect to be passed options[OWNER] so they can function properly. Examples are ember-glimmer/environment and ember-glimmer/template. These should be refactored away from OWNER being passed in, and this fallback should be removed.

A handful of tests were added / updated. In some cases _debugContainerKey assertions were removed, since this is no longer supported configuration (e.g. there is no way to inject _debugContainerKey onto the class itself without double extend).

[rwjblue]

Did a quick benchmark via ember-macro-benchmark to see if this has an effect on perf and it looks pretty good to me.

@krisselden / @stefanpenner - Mind confirming my interpretation?

Raw data: results-pr-15177.zip

[rwjblue]

I chatted with @krisselden in slack about the results, we came to roughly the same conclusion. That the changes in this PR do not reject the null hypothesis (that the variance between control and experiment are random) and therefore these changes do not represent a change in the performance profile of the application under test (emberaddons.com in this case).

[rwjblue]

Changes like this (changing the makeCtor functionality) should receive significant scrutiny, so I'd still like @stefanpenner and/or @krisselden to review this PR and approve....

[stefanpenner]

looks good, left some nit-picks.

[stefanpenner]

typeof this.class._initFactory === 'function' (more correct, and makes the JIT happy)

[stefanpenner]

550 and 551 are only required if this[NAME_KEY] isn't set, and it will be set most of the time. Can we avoid 550 and 551 unless we need to access it?

something like:

this[NAME_KEY] || meta(this).getFactory() || this.cosntructor.toString()

[stefanpenner]

Any preference to function accessors vs normal accessors? I can be fine with either, was just surprised this isn't a accessor and am curious if there was a reason for it

[rwjblue]

get / set seem fine to me, I just slightly dislike the babel transpilation (which requires the createClass helper):

// input

class Foo {
  get derp() { }
  set derp(value) { }
}

// output
var Foo = function () {
  function Foo() {
    _classCallCheck(this, Foo);
  }

  _createClass(Foo, [{
    key: "derp",
    get: function get() {},
    set: function set(value) {}
  }]);

  return Foo;
}();

[stefanpenner]

if we peekMeta meta may be undefined so we must guard before accessing getFactory

something like:

let meta = peekMeta(this)
if (meta === undefined) { return; }
let factory = meta.getFactory();
if (factory === undefined) { return ; }
return factory.fullName;

[stefanpenner]

ah, so this [OVERRIDE_OWNER] both in the setter and getter can be removed once the legacy support is dropped?

[rwjblue]

Yes, confirm

[krisselden]

confirm, OVERRIDE_OWNER is only for deprecated container setter

[stefanpenner]

Is it possible to have a ember-object without a meta? As we create its meta during within its constructor.

[krisselden]

it is not possible for it not to exists

[stefanpenner]

isn't it a tad strange to use peekMeta instead of meta ? But i'm fine if you are.

[stefanpenner]

Is it possible to have a ember-object without a meta? As we create its meta during within its constructor.

[krisselden]

it is not possible for it not to exists

[krisselden]

confirm, OVERRIDE_OWNER is only for deprecated container setter

[rwjblue]

Addressed concerns:

• Swapped usage of peekMeta to meta
• Moved Meta#getFactory / Meta#setFactory to simple getters
• Avoid accessing meta(this).factory when this[NAME_KEY] is present.

Anything else?