ui: Remove WithEventSource mixin, use a component instead (#7953)

The WithEventSource mixin was responsible for catching EventSource
errors and cleaning up events sources then the user left a Controller.

As we are trying to avoid mixin usage, we moved this all to an
`EventSource` component, which can clean up when the component is
removed from the page, and also fires an onerror event.

Moving to a component firing an onerror event means we can also remove
all of our custom computed property work that we were using previously
to catch errors (thrown when a service etc. is removed)

ui-v2/app/components/event-source/README.mdx

@@ -0,0 +1,25 @@
+## EventSource
+
+```handlebars
+<EventSource
+  @src={{eventSourceObject}}
+  @onerror={{action 'error'}}
+/>
+```
+
+### Arguments
+
+| Argument | Type | Default | Description |
+| --- | --- | --- | --- |
+| `src` | `EventSourceProxy` | | An EventSource object |
+| `onerror` | `Function` |  | The action to fire when an error occurs. Emits ErrorEvent object with an `error` property containing the Error. |
+| `closeOnDestroy` | `Boolean` | true | Whether to close and destroy the event source when the component is destroyed |
+
+This component is used to configure event source error from within a template, but also ensures that EventSources are cleaned/up destroyed when the user leaves the page (when the component is removed from the page)
+
+### See
+
+- [Component Source Code](./index.js)
+- [Template Source Code](./index.hbs)
+
+---

ui-v2/app/components/event-source/index.js

@@ -0,0 +1,38 @@
+import Component from '@ember/component';
+import { inject as service } from '@ember/service';
+
+export default Component.extend({
+  tagName: '',
+  dom: service('dom'),
+  logger: service('logger'),
+  closeOnDestroy: true,
+  onerror: function(e) {
+    this.logger.execute(e.error);
+  },
+  init: function() {
+    this._super(...arguments);
+    this._listeners = this.dom.listeners();
+  },
+  willDestroyElement: function() {
+    if (this.closeOnDestroy && typeof (this.src || {}).close === 'function') {
+      this.src.close();
+      this.src.willDestroy();
+    }
+    this._listeners.remove();
+    this._super(...arguments);
+  },
+  didReceiveAttrs: function() {
+    this._listeners.remove();
+    if (typeof (this.src || {}).addEventListener === 'function') {
+      this._listeners.add(this.src, {
+        error: e => {
+          try {
+            this.onerror(e);
+          } catch (err) {
+            this.logger.execute(e.error);
+          }
+        },
+      });
+    }
+  },
+});

ui-v2/app/components/tabular-details/index.hbs

@@ -7,6 +7,7 @@
     </tr>
   </thead>
   <tbody>
+  {{#let (concat 'tabular-details-' name '-toggle-' guid '_') as |inputId|}}
     {{#each items as |item index|}}
       <tr data-test-tabular-row onclick={{action 'click'}}>
           <YieldSlot @name="row">{{yield item index}}</YieldSlot>
@@ -28,5 +29,6 @@
         </td>
       </tr>
     {{/each}}
+  {{/let}}
   </tbody>
 </table>

ui-v2/app/components/tabular-details/index.js

@@ -1,20 +1,14 @@
 import Component from '@ember/component';
-import SlotsMixin from 'block-slots';
 import { inject as service } from '@ember/service';
-import { set } from '@ember/object';
+import Slotted from 'block-slots';
-import { subscribe } from 'consul-ui/utils/computed/purify';
 
-let uid = 0;
+export default Component.extend(Slotted, {
-export default Component.extend(SlotsMixin, {
   dom: service('dom'),
   onchange: function() {},
   init: function() {
     this._super(...arguments);
-    set(this, 'uid', uid++);
+    this.guid = this.dom.guid(this);
   },
-  inputId: subscribe('name', 'uid', function(name = 'name') {
-    return `tabular-details-${name}-toggle-${uid}_`;
-  }),
   actions: {
     click: function(e) {
       this.dom.clickFirstAnchor(e);

ui-v2/app/computed/catchable.js

@@ -1,11 +0,0 @@
-import { computed as computedPropertyFactory } from '@ember/object';
-
-export const computed = function() {
-  const prop = computedPropertyFactory(...arguments);
-  prop.catch = function(cb) {
-    return this.meta({
-      catch: cb,
-    });
-  };
-  return prop;
-};

ui-v2/app/controllers/dc/intentions/index.js

@@ -1,6 +1,5 @@
 import Controller from '@ember/controller';
-import WithEventSource from 'consul-ui/mixins/with-event-source';
+export default Controller.extend({
-export default Controller.extend(WithEventSource, {
   queryParams: {
     filterBy: {
       as: 'action',

ui-v2/app/controllers/dc/nodes/index.js

@@ -1,7 +1,6 @@
 import Controller from '@ember/controller';
-import WithEventSource from 'consul-ui/mixins/with-event-source';
 
-export default Controller.extend(WithEventSource, {
+export default Controller.extend({
   queryParams: {
     filterBy: {
       as: 'status',

ui-v2/app/controllers/dc/nodes/show.js

@@ -2,13 +2,13 @@ import Controller from '@ember/controller';
 import { inject as service } from '@ember/service';
 import { get } from '@ember/object';
 import { alias } from '@ember/object/computed';
-import WithEventSource, { listen } from 'consul-ui/mixins/with-event-source';
 
-export default Controller.extend(WithEventSource, {
+export default Controller.extend({
   dom: service('dom'),
   notify: service('flashMessages'),
   items: alias('item.Services'),
-  item: listen('item').catch(function(e) {
+  actions: {
+    error: function(e) {
       if (e.target.readyState === 1) {
         // OPEN
         if (get(e, 'error.errors.firstObject.status') === '404') {
@@ -18,9 +18,13 @@ export default Controller.extend(WithEventSource, {
             type: 'warning',
             action: 'update',
           });
-        this.tomography.close();
+          [e.target, this.tomography, this.sessions].forEach(function(item) {
-        this.sessions.close();
+            if (item && typeof item.close === 'function') {
+              item.close();
+            }
+          });
         }
       }
-  }),
+    },
+  },
 });

ui-v2/app/controllers/dc/nspaces/index.js

@@ -1,7 +1,6 @@
 import Controller from '@ember/controller';
 
-import WithEventSource from 'consul-ui/mixins/with-event-source';
+export default Controller.extend({
-export default Controller.extend(WithEventSource, {
   queryParams: {
     search: {
       as: 'filter',

ui-v2/app/controllers/dc/services/index.js

@@ -1,7 +1,7 @@
 import Controller from '@ember/controller';
 import { computed } from '@ember/object';
-import WithEventSource from 'consul-ui/mixins/with-event-source';
+
-export default Controller.extend(WithEventSource, {
+export default Controller.extend({
   queryParams: {
     sortBy: 'sort',
     search: {

ui-v2/app/controllers/dc/services/instance.js

@@ -1,11 +1,11 @@
 import Controller from '@ember/controller';
 import { get } from '@ember/object';
 import { inject as service } from '@ember/service';
-import WithEventSource, { listen } from 'consul-ui/mixins/with-event-source';
 
-export default Controller.extend(WithEventSource, {
+export default Controller.extend({
   notify: service('flashMessages'),
-  item: listen('item').catch(function(e) {
+  actions: {
+    error: function(e) {
       if (e.target.readyState === 1) {
         // OPEN
         if (get(e, 'error.errors.firstObject.status') === '404') {
@@ -15,11 +15,13 @@ export default Controller.extend(WithEventSource, {
             type: 'warning',
             action: 'update',
           });
-        const proxy = this.proxy;
+          [e.target, this.proxy].forEach(function(item) {
-        if (proxy) {
+            if (item && typeof item.close === 'function') {
-          proxy.close();
+              item.close();
+            }
+          });
         }
       }
-    }
+    },
-  }),
+  },
 });

ui-v2/app/controllers/dc/services/show.js

@@ -1,11 +1,11 @@
 import Controller from '@ember/controller';
 import { get } from '@ember/object';
 import { inject as service } from '@ember/service';
-import WithEventSource, { listen } from 'consul-ui/mixins/with-event-source';
+export default Controller.extend({
-export default Controller.extend(WithEventSource, {
   dom: service('dom'),
   notify: service('flashMessages'),
-  item: listen('item').catch(function(e) {
+  actions: {
+    error: function(e) {
       if (e.target.readyState === 1) {
         // OPEN
         if (get(e, 'error.errors.firstObject.status') === '404') {
@@ -16,6 +16,14 @@ export default Controller.extend(WithEventSource, {
             action: 'update',
           });
         }
+        [e.target, this.intentions, this.chain, this.proxies, this.gatewayServices].forEach(
+          function(item) {
+            if (item && typeof item.close === 'function') {
+              item.close();
             }
-  }),
+          }
+        );
+      }
+    },
+  },
 });

ui-v2/app/instance-initializers/event-source.js

@@ -19,7 +19,7 @@ export function initialize(container) {
       };
     })
     .concat(
-      ['dc', 'policy', 'role'].map(function(item) {
+      ['policy', 'role'].map(function(item) {
         // create repositories that return a promise resolving to an EventSource
         return {
           service: `repository/${item}/component`,

ui-v2/app/mixins/with-event-source.js

@@ -1,61 +0,0 @@
-import Mixin from '@ember/object/mixin';
-import { set } from '@ember/object';
-import { computed as catchable } from 'consul-ui/computed/catchable';
-import purify from 'consul-ui/utils/computed/purify';
-
-import WithListeners from 'consul-ui/mixins/with-listeners';
-const PREFIX = '_';
-export default Mixin.create(WithListeners, {
-  setProperties: function(model) {
-    const _model = {};
-    Object.keys(model).forEach(prop => {
-      // here (see comment below on deleting)
-      if (model[prop] && typeof model[prop].addEventListener === 'function') {
-        let meta;
-        // TODO: metaForProperty throws an error if the property is not
-        // computed-like, this is far from ideal but happy with this
-        // until we can find a better way in an ember post 2.18 world
-        // of finding out if a property is computed or not
-        // (or until we switch all this out for <DataSource /> compoments
-        try {
-          meta = this.constructor.metaForProperty(prop);
-        } catch (e) {
-          meta = {};
-        }
-        if (typeof meta.catch === 'function') {
-          _model[`${PREFIX}${prop}`] = model[prop];
-          this.listen(_model[`_${prop}`], 'error', meta.catch.bind(this));
-        } else {
-          _model[prop] = model[prop];
-        }
-      } else {
-        _model[prop] = model[prop];
-      }
-    });
-    return this._super(_model);
-  },
-  reset: function(exiting) {
-    Object.keys(this).forEach(prop => {
-      if (this[prop] && typeof this[prop].close === 'function') {
-        this[prop].willDestroy();
-        // ember doesn't delete on 'resetController' by default
-        // right now we only call reset when we are exiting, therefore a full
-        // setProperties will be called the next time we enter the Route so this
-        // is ok for what we need and means that the above conditional works
-        // as expected (see 'here' comment above)
-        // delete this[prop];
-        // TODO: Check that nulling this out instead of deleting is fine
-        // pretty sure it is as above is just a falsey check
-        set(this, prop, null);
-      }
-    });
-    return this._super(...arguments);
-  },
-  willDestroy: function() {
-    this.reset(true);
-    this._super(...arguments);
-  },
-});
-export const listen = purify(catchable, function(props) {
-  return props.map(item => `${PREFIX}${item}`);
-});

ui-v2/app/templates/dc/intentions/index.hbs

@@ -1,4 +1,5 @@
 {{title 'Intentions'}}
+<EventSource @src={{items}} />
 {{#let (filter-by "Action" "deny" items) as |denied|}}
   {{#let (selectable-key-values
       (array "" (concat "All (" items.length ")"))

ui-v2/app/templates/dc/nodes/index.hbs

@@ -1,4 +1,5 @@
 {{title 'Nodes'}}
+<EventSource @src={{items}} />
 {{#let (selectable-key-values
     (array "" "All (Any Status)")
     (array "critical" "Critical Checks")

ui-v2/app/templates/dc/nodes/show.hbs

@@ -1,4 +1,5 @@
 {{title item.Node}}
+<EventSource @src={{item}} @onerror={{action "error"}} />
 <AppView @class="node show">
     <BlockSlot @name="notification" as |status type|>
       {{!TODO: Move sessions to its own folder within nodes }}

ui-v2/app/templates/dc/nspaces/index.hbs

@@ -1,4 +1,5 @@
 {{title 'Namespaces'}}
+<EventSource @src={{items}} />
 <AppView @class="nspace list" @loading={{isLoading}}>
   <BlockSlot @name="notification" as |status type subject|>
     {{partial 'dc/nspaces/notifications'}}

ui-v2/app/templates/dc/services/index.hbs

@@ -1,4 +1,5 @@
 {{title 'Services'}}
+<EventSource @src={{items}} />
 {{#let (selectable-key-values
     (array "Name:asc" "A to Z")
     (array "Name:desc" "Z to A")

ui-v2/app/templates/dc/services/instance.hbs

@@ -1,4 +1,6 @@
 {{title item.ID}}
+<EventSource @src={{item}} @onerror={{action "error"}} />
+<EventSource @src={{proxy}} />
 <AppView @class="instance show">
     <BlockSlot @name="notification" as |status type|>
       {{partial 'dc/services/notifications'}}

ui-v2/app/templates/dc/services/show.hbs

@@ -1,4 +1,7 @@
 {{title item.Service.Service}}
+<EventSource @src={{item}} @onerror={{action "error"}} />
+<EventSource @src={{chain}} />
+<EventSource @src={{intentions}} />
 <AppView @class="service show">
   <BlockSlot @name="notification" as |status type|>
     {{partial 'dc/services/notifications'}}

ui-v2/app/utils/computed/purify.js

@@ -1,42 +0,0 @@
-import { get, computed } from '@ember/object';
-
-/**
- * Converts a conventional non-pure Ember `computed` function into a pure one
- * (see https://github.com/emberjs/rfcs/blob/be351b059f08ac0fe709bc7697860d5064717a7f/text/0000-tracked-properties.md#avoiding-dependency-hell)
- *
- * @param {function} computed - a computed function to 'purify' (convert to a pure function)
- * @param {function} filter - Optional string filter function to pre-process the names of computed properties
- * @returns {function} - A pure `computed` function
- */
-const _success = function(value) {
-  return value;
-};
-const purify = function(computed, filter = args => args) {
-  return function() {
-    let args = [...arguments];
-    let success = _success;
-    // pop the user function off the end
-    if (typeof args[args.length - 1] === 'function') {
-      success = args.pop();
-    }
-    args = filter(args);
-    // this is the 'conventional' `computed`
-    const cb = function(name) {
-      return success.apply(
-        this,
-        args.map(item => {
-          // Right now this just takes the first part of the path so:
-          // `items.[]` or `items.@each.prop` etc
-          // gives you `items` which is 'probably' what you expect
-          // it won't work with something like `item.objects.[]`
-          // it could potentially be made to do so, but we don't need that right now at least
-          return get(this, item.split('.')[0]);
-        })
-      );
-    };
-    // concat/push the user function back on
-    return computed(...args.concat([cb]));
-  };
-};
-export const subscribe = purify(computed);
-export default purify;

ui-v2/app/utils/dom/event-source/proxy.js

@@ -4,36 +4,24 @@ const proxies = {};
 export default function(ObjProxy, ArrProxy, createListeners) {
   return function(source, data = []) {
     let Proxy = ObjProxy;
-    // TODO: Why are these two separate?
+    // TODO: When is data ever a string?
-    // And when is data ever a string?
-    if (typeof data !== 'string' && typeof get(data, 'length') !== 'undefined') {
-      data = data.filter(function(item) {
-        return !get(item, 'isDestroyed') && !get(item, 'isDeleted') && get(item, 'isLoaded');
-      });
-    }
     let type = 'object';
     if (typeof data !== 'string' && typeof get(data, 'length') !== 'undefined') {
       Proxy = ArrProxy;
       type = 'array';
+      data = data.filter(function(item) {
+        return !get(item, 'isDestroyed') && !get(item, 'isDeleted') && get(item, 'isLoaded');
+      });
     }
     if (typeof proxies[type] === 'undefined') {
       proxies[type] = Proxy.extend({
-        closed: false,
-        error: null,
         init: function() {
           this.listeners = createListeners();
           this.listeners.add(this._source, 'message', e => set(this, 'content', e.data));
-          this.listeners.add(this._source, 'open', () => set(this, 'closed', false));
-          this.listeners.add(this._source, 'close', () => set(this, 'closed', true));
-          this.listeners.add(this._source, 'error', e => set(this, 'error', e.error));
           this._super(...arguments);
         },
         addEventListener: function(type, handler) {
-          // Force use of computed for messages
-          // Temporarily disable this restriction
-          // if (type !== 'message') {
           this.listeners.add(this._source, type, handler);
-          // }
         },
         getCurrentEvent: function() {
           return this._source.getCurrentEvent(...arguments);

ui-v2/tests/integration/components/event-source-test.js

@@ -0,0 +1,26 @@
+import { module, test } from 'qunit';
+import { setupRenderingTest } from 'ember-qunit';
+import { render } from '@ember/test-helpers';
+import { hbs } from 'ember-cli-htmlbars';
+
+module('Integration | Component | event-source', function(hooks) {
+  setupRenderingTest(hooks);
+
+  test('it renders', async function(assert) {
+    // Set any properties with this.set('myProperty', 'value');
+    // Handle any actions with this.set('myAction', function(val) { ... });
+
+    await render(hbs`<EventSource />`);
+
+    assert.equal(this.element.textContent.trim(), '');
+
+    // Template block usage:
+    await render(hbs`
+      <EventSource>
+        template block text
+      </EventSource>
+    `);
+
+    assert.equal(this.element.textContent.trim(), 'template block text');
+  });
+});