[WIP][BUGFIX] don't rely on elementId for event dispatching

packages/@ember/-internals/glimmer/lib/component-managers/curly.ts

@@ -2,7 +2,12 @@ import { privatize as P } from '@ember/-internals/container';
 import { get } from '@ember/-internals/metal';
 import { getOwner } from '@ember/-internals/owner';
 import { guidFor } from '@ember/-internals/utils';
-import { addChildView, OwnedTemplateMeta, setViewElement } from '@ember/-internals/views';
+import {
+  addChildView,
+  OwnedTemplateMeta,
+  setElementView,
+  setViewElement,
+} from '@ember/-internals/views';
 import { assert } from '@ember/debug';
 import { _instrumentStart } from '@ember/instrumentation';
 import { assign } from '@ember/polyfills';
@@ -318,6 +323,7 @@ export default class CurlyComponentManager
     operations: ElementOperations
   ): void {
     setViewElement(component, element);
+    setElementView(element, component);
 
     let { attributeBindings, classNames, classNameBindings } = component;
 

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

@@ -727,7 +727,15 @@ const Component = CoreView.extend(
      */
     readDOMAttr(name: string) {
       // TODO revisit this
-      let element = getViewElement(this);
+      let _element = getViewElement(this);
+
+      assert(
+        `Cannot call \`readDOMAttr\` on ${this} which does not have an element`,
+        _element !== null
+      );
+
+      let element = _element!;
+
       let isSVG = element.namespaceURI === SVG_NAMESPACE;
       let { type, normalized } = normalizeProperty(element, name);
 

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

@@ -1,14 +1,9 @@
 import { ENV } from '@ember/-internals/environment';
 import { runInTransaction } from '@ember/-internals/metal';
-import {
-  fallbackViewRegistry,
-  getViewElement,
-  getViewId,
-  setViewElement,
-} from '@ember/-internals/views';
+import { clearViewElement, getViewElement, getViewId } from '@ember/-internals/views';
 import { assert } from '@ember/debug';
 import { backburner, getCurrentRunLoop } from '@ember/runloop';
-import { Simple } from '@glimmer/interfaces';
+import { Option, Simple } from '@glimmer/interfaces';
 import { CURRENT_TAG, VersionedPathReference } from '@glimmer/reference';
 import {
   clientBuilder,
@@ -245,12 +240,14 @@ function loopEnd() {
 backburner.on('begin', loopBegin);
 backburner.on('end', loopEnd);
 
+interface ViewRegistry {
+  [viewId: string]: Opaque;
+}
+
 export abstract class Renderer {
   private _env: Environment;
   private _rootTemplate: any;
-  private _viewRegistry: {
-    [viewId: string]: Opaque;
-  };
+  private _viewRegistry: ViewRegistry;
   private _destinedForDOM: boolean;
   private _destroyed: boolean;
   private _roots: RootState[];
@@ -262,13 +259,13 @@ export abstract class Renderer {
   constructor(
     env: Environment,
     rootTemplate: OwnedTemplate,
-    _viewRegistry = fallbackViewRegistry,
+    viewRegistry: ViewRegistry,
     destinedForDOM = false,
     builder = clientBuilder
   ) {
     this._env = env;
     this._rootTemplate = rootTemplate;
-    this._viewRegistry = _viewRegistry;
+    this._viewRegistry = viewRegistry;
     this._destinedForDOM = destinedForDOM;
     this._destroyed = false;
     this._roots = [];
@@ -331,15 +328,9 @@ export abstract class Renderer {
 
     this.cleanupRootFor(view);
 
-    setViewElement(view, null);
-
     if (this._destinedForDOM) {
       view.trigger('didDestroyElement');
     }
-
-    if (!view.isDestroying) {
-      view.destroy();
-    }
   }
 
   cleanupRootFor(view: Opaque) {
@@ -370,7 +361,7 @@ export abstract class Renderer {
     this._clearAllRoots();
   }
 
-  abstract getElement(view: Opaque): Simple.Element | undefined;
+  abstract getElement(view: Opaque): Option<Simple.Element>;
 
   getBounds(view: Component) {
     let bounds = view[BOUNDS];
@@ -533,7 +524,7 @@ export class InertRenderer extends Renderer {
     return new this(env, rootTemplate, _viewRegistry, false, builder);
   }
 
-  getElement(_view: Opaque): Simple.Element | undefined {
+  getElement(_view: Opaque): Option<Simple.Element> {
     throw new Error(
       'Accessing `this.element` is not allowed in non-interactive environments (such as FastBoot).'
     );
@@ -555,7 +546,7 @@ export class InteractiveRenderer extends Renderer {
     return new this(env, rootTemplate, _viewRegistry, true, builder);
   }
 
-  getElement(view: Opaque): Simple.Element | undefined {
+  getElement(view: Opaque): Option<Simple.Element> {
     return getViewElement(view);
   }
 }

packages/@ember/-internals/glimmer/lib/utils/curly-component-state-bucket.ts

@@ -1,3 +1,4 @@
+import { clearElementView, clearViewElement, getViewElement } from '@ember/-internals/views';
 import { Revision, VersionedReference } from '@glimmer/reference';
 import { CapturedNamedArguments } from '@glimmer/runtime';
 import { Opaque } from '@glimmer/util';
@@ -54,6 +55,13 @@ export default class ComponentStateBucket {
     if (environment.isInteractive) {
       component.trigger('willDestroyElement');
       component.trigger('willClearRender');
+
+      let element = getViewElement(component);
+
+      if (element) {
+        clearElementView(element);
+        clearViewElement(component);
+      }
     }
 
     environment.destroyedComponents.push(component);

packages/@ember/-internals/views/index.d.ts

@@ -1,4 +1,4 @@
-import { Simple, Template } from '@glimmer/interfaces';
+import { Simple, Template, Option } from '@glimmer/interfaces';
 import { Opaque } from '@glimmer/util';
 import { Factory, Owner } from '@ember/-internals/owner';
 
@@ -21,8 +21,12 @@ export const ViewMixin: any;
 export const ViewStateSupport: any;
 export const TextSupport: any;
 
-export function getViewElement(view: Opaque): Simple.Element;
-export function setViewElement(view: Opaque, element: Simple.Element | null): void;
+export function getElementView(element: Simple.Element): Opaque;
+export function getViewElement(view: Opaque): Option<Simple.Element>;
+export function setElementView(element: Simple.Element, view: Opaque): void;
+export function setViewElement(view: Opaque, element: Simple.Element): void;
+export function clearElementView(element: Simple.Element): void;
+export function clearViewElement(view: Opaque): void;
 
 export function addChildView(parent: Opaque, child: Opaque): void;
 
@@ -45,10 +49,6 @@ export function lookupPartial(templateName: string, owner: Owner): any;
 
 export function getViewId(view: any): string;
 
-export const fallbackViewRegistry: {
-  [id: string]: any | undefined;
-};
-
 export const MUTABLE_CELL: string;
 
 export const ActionManager: {

packages/@ember/-internals/views/index.js

@@ -8,8 +8,12 @@ export {
   getRootViews,
   getChildViews,
   getViewId,
+  getElementView,
   getViewElement,
+  setElementView,
   setViewElement,
+  clearElementView,
+  clearViewElement,
   constructStyleDeprecationMessage,
 } from './lib/system/utils';
 export { default as EventDispatcher } from './lib/system/event_dispatcher';
@@ -25,4 +29,3 @@ export { MUTABLE_CELL } from './lib/compat/attrs';
 export { default as lookupPartial, hasPartial } from './lib/system/lookup_partial';
 export { default as lookupComponent } from './lib/utils/lookup-component';
 export { default as ActionManager } from './lib/system/action_manager';
-export { default as fallbackViewRegistry } from './lib/compat/fallback-view-registry';

packages/@ember/-internals/views/lib/system/event_dispatcher.js

@@ -3,9 +3,9 @@ import { assign } from '@ember/polyfills';
 import { assert } from '@ember/debug';
 import { get, set } from '@ember/-internals/metal';
 import { Object as EmberObject } from '@ember/-internals/runtime';
+import { getElementView } from '@ember/-internals/views';
 import jQuery, { jQueryDisabled } from './jquery';
 import ActionManager from './action_manager';
-import fallbackViewRegistry from '../compat/fallback-view-registry';
 import addJQueryEventDeprecation from './jquery_event_deprecation';
 import { contains } from './utils';
 import { JQUERY_INTEGRATION } from '@ember/deprecated-features';
@@ -216,11 +216,9 @@ export default EmberObject.extend({
       }
     }
 
-    let viewRegistry = this._getViewRegistry();
-
     for (let event in events) {
       if (events.hasOwnProperty(event)) {
-        this.setupHandler(rootElement, event, events[event], viewRegistry);
+        this.setupHandler(rootElement, event, events[event]);
       }
     }
   },
@@ -238,16 +236,15 @@ export default EmberObject.extend({
     @param {Element} rootElement
     @param {String} event the browser-originated event to listen to
     @param {String} eventName the name of the method to call on the view
-    @param {Object} viewRegistry
   */
-  setupHandler(rootElement, event, eventName, viewRegistry) {
+  setupHandler(rootElement, event, eventName) {
     if (eventName === null) {
       return;
     }
 
     if (!JQUERY_INTEGRATION || jQueryDisabled) {
       let viewHandler = (target, event) => {
-        let view = viewRegistry[target.id];
+        let view = getElementView(target);
         let result = true;
 
         if (view) {
@@ -340,7 +337,7 @@ export default EmberObject.extend({
             (related === null || (related !== target && !contains(target, related)))
           ) {
             // mouseEnter/Leave don't bubble, so there is no logic to prevent it as with other events
-            if (viewRegistry[target.id]) {
+            if (getElementView(target)) {
               viewHandler(target, createFakeEvent(origEventType, event));
             } else if (target.hasAttribute('data-ember-action')) {
               actionHandler(target, createFakeEvent(origEventType, event));
@@ -358,7 +355,7 @@ export default EmberObject.extend({
           let target = event.target;
 
           do {
-            if (viewRegistry[target.id]) {
+            if (getElementView(target)) {
               if (viewHandler(target, event) === false) {
                 event.preventDefault();
                 event.stopPropagation();
@@ -378,7 +375,7 @@ export default EmberObject.extend({
       }
     } else {
       rootElement.on(`${event}.ember`, '.ember-view', function(evt) {
-        let view = viewRegistry[this.id];
+        let view = getElementView(this);
         let result = true;
 
         if (view) {
@@ -417,13 +414,6 @@ export default EmberObject.extend({
     }
   },
 
-  _getViewRegistry() {
-    let owner = getOwner(this);
-    let viewRegistry = (owner && owner.lookup('-view-registry:main')) || fallbackViewRegistry;
-
-    return viewRegistry;
-  },
-
   destroy() {
     let rootElementSelector = get(this, 'rootElement');
     let rootElement;

packages/@ember/-internals/views/lib/system/utils.js

@@ -1,6 +1,6 @@
 import { getOwner } from '@ember/-internals/owner';
 /* globals Element */
-import { guidFor, symbol } from '@ember/-internals/utils';
+import { guidFor } from '@ember/-internals/utils';
 
 /**
 @module ember
@@ -60,23 +60,38 @@ export function getViewId(view) {
   }
 }
 
-const VIEW_ELEMENT = symbol('VIEW_ELEMENT');
+const ELEMENT_VIEW = new WeakMap();
+const VIEW_ELEMENT = new WeakMap();
+
+export function getElementView(element) {
+  return ELEMENT_VIEW.get(element) || null;
+}
 
 /**
   @private
   @method getViewElement
   @param {Ember.View} view
  */
 export function getViewElement(view) {
-  return view[VIEW_ELEMENT];
+  return VIEW_ELEMENT.get(view) || null;
 }
 
-export function initViewElement(view) {
-  view[VIEW_ELEMENT] = null;
+export function setElementView(element, view) {
+  ELEMENT_VIEW.set(element, view);
 }
 
 export function setViewElement(view, element) {
-  return (view[VIEW_ELEMENT] = element);
+  VIEW_ELEMENT.set(view, element);
+}
+
+// These are not needed for GC, but for correctness
+
+export function clearElementView(element) {
+  ELEMENT_VIEW.delete(element);
+}
+
+export function clearViewElement(view) {
+  VIEW_ELEMENT.delete(view);
 }
 
 const CHILD_VIEW_IDS = new WeakMap();

packages/@ember/-internals/views/lib/views/core_view.js

@@ -1,5 +1,4 @@
 import { ActionHandler, Evented, FrameworkObject } from '@ember/-internals/runtime';
-import { initViewElement } from '../system/utils';
 import states from './states';
 
 /**
@@ -28,8 +27,6 @@ const CoreView = FrameworkObject.extend(Evented, ActionHandler, {
     this._state = 'preRender';
     this._currentState = this._states.preRender;
 
-    initViewElement(this);
-
     if (!this.renderer) {
       throw new Error(
         `Cannot instantiate a component without a renderer. Please ensure that you are creating ${this} with a proper container/registry.`

packages/@ember/engine/index.js

@@ -497,7 +497,7 @@ function commonSetupRegistry(registry) {
 
   registry.injection('view', '_viewRegistry', '-view-registry:main');
   registry.injection('renderer', '_viewRegistry', '-view-registry:main');
-  registry.injection('event_dispatcher:main', '_viewRegistry', '-view-registry:main');
+  registry.injection('event_dispatcher', '_viewRegistry', '-view-registry:main');
 
   registry.injection('route', '_topLevelViewTemplate', 'template:-outlet');
 

packages/internal-test-helpers/lib/test-cases/abstract-rendering.js

@@ -21,12 +21,18 @@ export default class AbstractRenderingTestCase extends AbstractTestCase {
       bootOptions,
     }));
 
+    owner.register('-view-registry:main', Object.create(null), { instantiate: false });
+    owner.register('event_dispatcher:main', EventDispatcher);
+
+    // TODO: why didn't buildOwner do this for us?
+    owner.inject('view', '_viewRegistry', '-view-registry:main');
+    owner.inject('renderer', '_viewRegistry', '-view-registry:main');
+    owner.inject('event_dispatcher', '_viewRegistry', '-view-registry:main');
+
     this.renderer = this.owner.lookup('renderer:-dom');
     this.element = document.querySelector('#qunit-fixture');
     this.component = null;
 
-    owner.register('event_dispatcher:main', EventDispatcher);
-    owner.inject('event_dispatcher:main', '_viewRegistry', '-view-registry:main');
     if (!bootOptions || bootOptions.isInteractive !== false) {
       owner.lookup('event_dispatcher:main').setup(this.getCustomDispatcherEvents(), this.element);
     }