Merge pull request #17818 from emberjs/fix-event-dispatcher-1

Fix event dispatcher (part 1)
emberjs · Mar 29, 2019 · e0be0db · e0be0db
2 parents 8ad9bbc + d1688b5
commit e0be0db
Show file tree

Hide file tree

Showing 20 changed files with 243 additions and 212 deletions.
diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/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, debugFreeze } from '@ember/debug';
 import { _instrumentStart } from '@ember/instrumentation';
 import { assign } from '@ember/polyfills';
@@ -340,6 +345,7 @@ export default class CurlyComponentManager
     operations: ElementOperations
   ): void {
     setViewElement(component, element);
+    setElementView(element, component);
 
     let { attributeBindings, classNames, classNameBindings } = component;
 

diff --git a/packages/@ember/-internals/glimmer/lib/component.ts b/packages/@ember/-internals/glimmer/lib/component.ts
@@ -725,7 +725,14 @@ 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);
 

diff --git a/packages/@ember/-internals/glimmer/lib/renderer.ts b/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 { 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);
   }
 }
diff --git a/packages/@ember/-internals/glimmer/lib/utils/curly-component-state-bucket.ts b/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);

diff --git a/...s/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js b/...s/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js
@@ -94,28 +94,54 @@ moduleFor(
         content: 'bizz bizz',
       });
 
-      runTask(() => this.rerender());
+      this.assertStableRerender();
+
+      runTask(() => set(this.context, 'customId', 'bar'));
+
+      this.assertComponentElement(this.firstChild, {
+        tagName: 'div',
+        attrs: { id: 'bizz' },
+        content: 'bar bizz',
+      });
+
+      runTask(() => set(this.context, 'customId', 'bizz'));
 
       this.assertComponentElement(this.firstChild, {
         tagName: 'div',
         attrs: { id: 'bizz' },
         content: 'bizz bizz',
       });
+    }
 
-      runTask(() => set(this.context, 'customId', 'bar'));
+    '@test it can have a custom id attribute and it is bound'() {
+      this.registerComponent('foo-bar', { template: 'hello' });
+
+      this.render('<FooBar id={{customId}} />', {
+        customId: 'bizz',
+      });
 
       this.assertComponentElement(this.firstChild, {
         tagName: 'div',
         attrs: { id: 'bizz' },
-        content: 'bar bizz',
+        content: 'hello',
+      });
+
+      this.assertStableRerender();
+
+      runTask(() => set(this.context, 'customId', 'bar'));
+
+      this.assertComponentElement(this.firstChild, {
+        tagName: 'div',
+        attrs: { id: 'bar' },
+        content: 'hello',
       });
 
       runTask(() => set(this.context, 'customId', 'bizz'));
 
       this.assertComponentElement(this.firstChild, {
         tagName: 'div',
         attrs: { id: 'bizz' },
-        content: 'bizz bizz',
+        content: 'hello',
       });
     }