address comments

packages/@ember/-internals/container/lib/container.ts

@@ -246,7 +246,7 @@ if (DEBUG) {
  * Wrap a factory manager in a proxy which will not permit properties to be
  * set on the manager.
  */
-function wrapManagerInDeprecationProxy<T, C>(manager: FactoryManager<T, C>) {
+function wrapManagerInDeprecationProxy<T, C>(manager: FactoryManager<T, C>): FactoryManager<T, C> {
   if (HAS_NATIVE_PROXY) {
     let validator = {
       set(_obj: T, prop: keyof T) {
@@ -267,8 +267,7 @@ function wrapManagerInDeprecationProxy<T, C>(manager: FactoryManager<T, C>) {
       },
     };
 
-    let proxy = new Proxy(proxiedManager, validator as any);
-    // FACTORY_FOR.set(proxy, manager);
+    return new Proxy(proxiedManager, validator as any) as any;
   }
 
   return manager;
@@ -434,7 +433,7 @@ function instantiateFactory(
 }
 
 interface BuildInjectionsResult {
-  injections: { [key: string]: object } | undefined;
+  injections: { [key: string]: unknown };
   isDynamic: boolean;
 }
 
@@ -448,9 +447,6 @@ function processInjections(
   }
 
   let hash = result.injections;
-  if (hash === undefined) {
-    hash = result.injections = {};
-  }
 
   for (let i = 0; i < injections.length; i++) {
     let { property, specifier, source } = injections[i];
@@ -472,8 +468,12 @@ function buildInjections(
   typeInjections: Injection[],
   injections: Injection[]
 ): BuildInjectionsResult {
+  let injectionsHash = {};
+
+  setOwner(injectionsHash, container.owner!);
+
   let result: BuildInjectionsResult = {
-    injections: undefined,
+    injections: injectionsHash,
     isDynamic: false,
   };
 
@@ -560,7 +560,7 @@ class FactoryManager<T, C> {
   readonly fullName: string;
   readonly normalizedName: string;
   private madeToString: string | undefined;
-  injections: { [key: string]: object } | undefined;
+  injections: { [key: string]: unknown } | undefined;
 
   constructor(
     container: Container,
@@ -598,9 +598,8 @@ class FactoryManager<T, C> {
     let props = this.injections;
     if (props === undefined) {
       let { injections, isDynamic } = injectionsFor(this.container, this.normalizedName);
-      props = injections || {};
-      setOwner(props, this.owner!);
-      props[INIT_FACTORY as string] = this;
+      setFactoryFor(injections, this);
+      props = injections;
 
       if (!isDynamic) {
         this.injections = injections;
@@ -628,11 +627,10 @@ class FactoryManager<T, C> {
 
       validationCache[this.fullName] = true;
 
-      if (!this.class.create) {
-        throw new Error(
-          `Failed to create an instance of '${this.normalizedName}'. Most likely an improperly defined class or an invalid module export.`
-        );
-      }
+      assert(
+        `Failed to create an instance of '${this.normalizedName}'. Most likely an improperly defined class or an invalid module export.`,
+        typeof this.class.create === 'function'
+      );
     }
 
     return this.class.create(props);

packages/@ember/-internals/container/tests/container_test.js

@@ -190,13 +190,13 @@ moduleFor(
       assert.equal(container.lookup('doesnot:exist'), undefined);
     }
 
-    ['@test An invalid factory throws an error'](assert) {
+    ['@test An invalid factory throws an error']() {
       let registry = new Registry();
       let container = registry.container();
 
       registry.register('controller:foo', {});
 
-      assert.throws(() => {
+      expectAssertion(() => {
         container.lookup('controller:foo');
       }, /Failed to create an instance of 'controller:foo'/);
     }

packages/@ember/-internals/container/tests/owner_test.js

@@ -25,6 +25,8 @@ moduleFor(
 
       let legacyOwner;
 
+      // This is not something we expect to happen a lot, but does exist currently
+      // in the wild: https://github.com/hjdivad/ember-m3/pull/822
       for (let key in obj) {
         legacyOwner = key;
       }

packages/@ember/-internals/glimmer/lib/renderer.ts

@@ -1,5 +1,5 @@
 import { ENV } from '@ember/-internals/environment';
-import { OWNER, Owner } from '@ember/-internals/owner';
+import { getOwner, Owner } from '@ember/-internals/owner';
 import { getViewElement, getViewId, OwnedTemplateMeta } from '@ember/-internals/views';
 import { assert } from '@ember/debug';
 import { backburner, getCurrentRunLoop } from '@ember/runloop';
@@ -543,22 +543,15 @@ export abstract class Renderer {
 }
 
 export class InertRenderer extends Renderer {
-  static create({
-    [OWNER]: owner,
-    document,
-    env,
-    rootTemplate,
-    _viewRegistry,
-    builder,
-  }: {
-    [OWNER]: Owner;
+  static create(props: {
     document: SimpleDocument;
     env: { isInteractive: boolean; hasDOM: boolean };
     rootTemplate: TemplateFactory;
     _viewRegistry: any;
     builder: any;
   }) {
-    return new this(owner, document, env, rootTemplate, _viewRegistry, false, builder);
+    let { document, env, rootTemplate, _viewRegistry, builder } = props;
+    return new this(getOwner(props), document, env, rootTemplate, _viewRegistry, false, builder);
   }
 
   getElement(_view: unknown): Option<SimpleElement> {
@@ -569,22 +562,15 @@ export class InertRenderer extends Renderer {
 }
 
 export class InteractiveRenderer extends Renderer {
-  static create({
-    [OWNER]: owner,
-    document,
-    env,
-    rootTemplate,
-    _viewRegistry,
-    builder,
-  }: {
-    [OWNER]: Owner;
+  static create(props: {
     document: SimpleDocument;
     env: { isInteractive: boolean; hasDOM: boolean };
     rootTemplate: TemplateFactory;
     _viewRegistry: any;
     builder: any;
   }) {
-    return new this(owner, document, env, rootTemplate, _viewRegistry, true, builder);
+    let { document, env, rootTemplate, _viewRegistry, builder } = props;
+    return new this(getOwner(props), document, env, rootTemplate, _viewRegistry, true, builder);
   }
 
   getElement(view: unknown): Option<SimpleElement> {

packages/@ember/-internals/glimmer/lib/views/outlet.ts

@@ -1,4 +1,4 @@
-import { OWNER, Owner } from '@ember/-internals/owner';
+import { getOwner, Owner } from '@ember/-internals/owner';
 import { assign } from '@ember/polyfills';
 import { schedule } from '@ember/runloop';
 import { SimpleElement } from '@simple-dom/interface';
@@ -35,7 +35,7 @@ export default class OutletView {
 
   static create(options: any) {
     let { _environment, renderer, template: templateFactory } = options;
-    let owner = options[OWNER];
+    let owner = getOwner(options);
     let template = templateFactory(owner);
     return new OutletView(_environment, renderer, owner, template);
   }

packages/@ember/-internals/metal/lib/events.ts

@@ -146,11 +146,13 @@ export function sendEvent(
     if (!target) {
       target = obj;
     }
-    if (typeof method === 'string' || typeof method === 'symbol') {
-      method = target[method] as Function;
+
+    let type = typeof method;
+    if (type === 'string' || type === 'symbol') {
+      method = target[method as string] as Function;
     }
 
-    method.apply(target, params);
+    (method as Function).apply(target, params);
   }
   return true;
 }

packages/@ember/-internals/owner/index.ts

@@ -40,7 +40,7 @@ export interface Owner {
 import { enumerableSymbol, symbol } from '@ember/-internals/utils';
 import { deprecate } from '@ember/debug';
 
-export const LEGACY_OWNER: unique symbol = enumerableSymbol('OWNER') as any;
+export const LEGACY_OWNER: unique symbol = enumerableSymbol('LEGACY_OWNER') as any;
 export const OWNER: unique symbol = symbol('OWNER') as any;
 
 /**

packages/@ember/-internals/runtime/lib/system/core_object.js

@@ -2,15 +2,16 @@
   @module @ember/object
 */
 
-import { getFactoryFor, setFactoryFor } from '@ember/-internals/container';
-import { getOwner, OWNER } from '@ember/-internals/owner';
+import { getFactoryFor, setFactoryFor, INIT_FACTORY } from '@ember/-internals/container';
+import { getOwner, LEGACY_OWNER, OWNER } from '@ember/-internals/owner';
 import { assign, _WeakSet as WeakSet } from '@ember/polyfills';
 import {
   guidFor,
   getName,
   setName,
   makeArray,
   HAS_NATIVE_PROXY,
+  HAS_NATIVE_SYMBOL,
   isInternalSymbol,
 } from '@ember/-internals/utils';
 import { meta } from '@ember/-internals/meta';
@@ -196,7 +197,9 @@ function initialize(obj, properties) {
 */
 class CoreObject {
   constructor(owner) {
-    // pluck off factory and set it as the instance factory
+    // setOwner has to set both OWNER and LEGACY_OWNER for backwards compatibility, and
+    // LEGACY_OWNER is enumerable, so setting it would add an enumerable property to the object,
+    // so we just set `OWNER` directly here.
     this[OWNER] = owner;
 
     // prepare prototype...
@@ -266,6 +269,10 @@ class CoreObject {
     }
   }
 
+  // Empty setter for absorbing setting the LEGACY_OWNER, which should _not_
+  // become an enumerable property, and should not be used in general.
+  set [LEGACY_OWNER](value) {}
+
   reopen(...args) {
     applyMixin(this, args);
     return this;
@@ -1068,4 +1075,30 @@ if (DEBUG) {
   };
 }
 
+if (!HAS_NATIVE_SYMBOL) {
+  // Allows OWNER and INIT_FACTORY to be non-enumerable in IE11
+  let instanceOwner = new WeakMap();
+  let instanceFactory = new WeakMap();
+
+  Object.defineProperty(CoreObject.prototype, OWNER, {
+    get() {
+      return instanceOwner.get(this);
+    },
+
+    set(value) {
+      instanceOwner.set(this, value);
+    },
+  });
+
+  Object.defineProperty(CoreObject.prototype, INIT_FACTORY, {
+    get() {
+      return instanceFactory.get(this);
+    },
+
+    set(value) {
+      instanceFactory.set(this, value);
+    },
+  });
+}
+
 export default CoreObject;

packages/@ember/-internals/runtime/lib/system/object.js

@@ -2,9 +2,8 @@
 @module @ember/object
 */
 
-import { getFactoryFor, INIT_FACTORY } from '@ember/-internals/container';
-import { OWNER, setOwner } from '@ember/-internals/owner';
-import { HAS_NATIVE_SYMBOL, symbol, setName } from '@ember/-internals/utils';
+import { getFactoryFor } from '@ember/-internals/container';
+import { symbol, setName } from '@ember/-internals/utils';
 import { addListener } from '@ember/-internals/metal';
 import CoreObject from './core_object';
 import Observable from '../mixins/observable';
@@ -28,32 +27,6 @@ export default class EmberObject extends CoreObject {
   }
 }
 
-if (!HAS_NATIVE_SYMBOL) {
-  // Allows OWNER and INIT_FACTORY to be non-enumerable in IE11
-  let instanceOwner = new WeakMap();
-  let instanceFactory = new WeakMap();
-
-  Object.defineProperty(EmberObject.prototype, OWNER, {
-    get() {
-      return instanceOwner.get(this);
-    },
-
-    set(value) {
-      instanceOwner.set(this, value);
-    },
-  });
-
-  Object.defineProperty(EmberObject.prototype, INIT_FACTORY, {
-    get() {
-      return instanceFactory.get(this);
-    },
-
-    set(value) {
-      instanceFactory.set(this, value);
-    },
-  });
-}
-
 setName(EmberObject, 'Ember.Object');
 
 Observable.apply(EmberObject.prototype);
@@ -65,12 +38,6 @@ FrameworkObject = class FrameworkObject extends CoreObject {
     let factory = getFactoryFor(this);
     return factory !== undefined && factory.fullName;
   }
-
-  constructor(owner) {
-    super();
-
-    setOwner(this, owner);
-  }
 };
 
 Observable.apply(FrameworkObject.prototype);

packages/@ember/-internals/runtime/tests/system/core_object_test.js

@@ -1,7 +1,7 @@
 import { getOwner, setOwner } from '@ember/-internals/owner';
-import { get, set } from '@ember/-internals/metal';
+import { get, set, observer } from '@ember/-internals/metal';
 import CoreObject from '../../lib/system/core_object';
-import { moduleFor, AbstractTestCase, buildOwner } from 'internal-test-helpers';
+import { moduleFor, AbstractTestCase, buildOwner, runLoopSettled } from 'internal-test-helpers';
 import { track } from '@glimmer/validator';
 
 moduleFor(
@@ -85,6 +85,32 @@ moduleFor(
       }).create(options);
     }
 
+    async ['@test observed properties are enumerable when set GH#14594'](assert) {
+      let callCount = 0;
+      let Test = CoreObject.extend({
+        myProp: null,
+        anotherProp: undefined,
+        didChangeMyProp: observer('myProp', function() {
+          callCount++;
+        }),
+      });
+
+      let test = Test.create();
+      set(test, 'id', '3');
+      set(test, 'myProp', { id: 1 });
+
+      assert.deepEqual(Object.keys(test).sort(), ['id', 'myProp']);
+
+      set(test, 'anotherProp', 'nice');
+
+      assert.deepEqual(Object.keys(test).sort(), ['anotherProp', 'id', 'myProp']);
+      await runLoopSettled();
+
+      assert.equal(callCount, 1);
+
+      test.destroy();
+    }
+
     ['@test native getters/setters do not cause rendering invalidation during init'](assert) {
       let objectMeta = Object.create(null);