Skip to content

Commit

Permalink
ui: Change vocab of ReopenableEventSource from 'reopen' to 'open' (#5973
Browse files Browse the repository at this point in the history
)

You can potentially close an EventSource before its first tick by
immediately setting the readyState to a non-open state. Therefore it
never opens.

Calling `open` will then open it.

'Open' fits better than 'reopen' when taking the above into account
  • Loading branch information
johncowen authored and John Cowen committed Jun 21, 2019
1 parent ae17676 commit 09d6cfb
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 21 deletions.
6 changes: 3 additions & 3 deletions ui-v2/app/utils/dom/event-source/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import proxyFactory from 'consul-ui/utils/dom/event-source/proxy';
import firstResolverFactory from 'consul-ui/utils/dom/event-source/resolver';

import CallableEventSourceFactory from 'consul-ui/utils/dom/event-source/callable';
import ReopenableEventSourceFactory from 'consul-ui/utils/dom/event-source/reopenable';
import OpenableEventSourceFactory from 'consul-ui/utils/dom/event-source/openable';
import BlockingEventSourceFactory from 'consul-ui/utils/dom/event-source/blocking';
import StorageEventSourceFactory from 'consul-ui/utils/dom/event-source/storage';

Expand Down Expand Up @@ -70,8 +70,8 @@ switch (env('CONSUL_UI_REALTIME_RUNNER')) {

// All The EventSource-i
export const CallableEventSource = CallableEventSourceFactory(EventTarget, Promise, runner);
export const ReopenableEventSource = ReopenableEventSourceFactory(CallableEventSource);
export const BlockingEventSource = BlockingEventSourceFactory(ReopenableEventSource);
export const OpenableEventSource = OpenableEventSourceFactory(CallableEventSource);
export const BlockingEventSource = BlockingEventSourceFactory(OpenableEventSource);
export const StorageEventSource = StorageEventSourceFactory(EventTarget, Promise);

// various utils
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default function(eventSource = EventSource) {
super(...arguments);
this.configuration = configuration;
}
reopen() {
open() {
switch (this.readyState) {
case 3: // CLOSING
this.readyState = 1;
Expand All @@ -18,6 +18,7 @@ export default function(eventSource = EventSource) {
eventSource.apply(this, [this.source, this.configuration]);
break;
}
return this;
}
};
}
4 changes: 2 additions & 2 deletions ui-v2/app/utils/dom/event-source/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export default function(ObjProxy, ArrProxy, createListeners) {
close: function() {
return source.close(...arguments);
},
reopen: function() {
return source.reopen(...arguments);
open: function() {
return source.open(...arguments);
},
willDestroy: function() {
this.listeners.remove();
Expand Down
2 changes: 1 addition & 1 deletion ui-v2/app/utils/dom/event-source/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default function(P = Promise) {
if (current != null) {
// immediately resolve if we have previous cached data
return P.resolve(current.data).then(function(cached) {
source.reopen();
source.open();
return cached;
});
}
Expand Down
2 changes: 1 addition & 1 deletion ui-v2/app/utils/dom/event-source/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default function(EventTarget, P = Promise) {
this.configuration = configuration;
this.configuration.cursor = 1;
this.dispatcher = configuration.dispatcher;
this.reopen();
this.open();
}
dispatchEvent() {
if (this.readyState === 1) {
Expand Down
4 changes: 2 additions & 2 deletions ui-v2/tests/unit/utils/dom/event-source/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
cache,
resolve,
CallableEventSource,
ReopenableEventSource,
OpenableEventSource,
BlockingEventSource,
StorageEventSource,
} from 'consul-ui/utils/dom/event-source/index';
Expand All @@ -16,7 +16,7 @@ module('Unit | Utility | dom/event source/index');
test('it works', function(assert) {
// All The EventSource
assert.ok(typeof CallableEventSource === 'function');
assert.ok(typeof ReopenableEventSource === 'function');
assert.ok(typeof OpenableEventSource === 'function');
assert.ok(typeof BlockingEventSource === 'function');
assert.ok(typeof StorageEventSource === 'function');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import domEventSourceReopenable from 'consul-ui/utils/dom/event-source/reopenable';
import domEventSourceOpenable from 'consul-ui/utils/dom/event-source/openable';
import { module } from 'qunit';
import test from 'ember-sinon-qunit/test-support/test';

module('Unit | Utility | dom/event-source/reopenable');
module('Unit | Utility | dom/event-source/openable');

const createEventSource = function() {
return class {
Expand All @@ -17,30 +17,30 @@ const createEventSource = function() {
close() {}
};
};
test('it creates an Reopenable class implementing EventSource', function(assert) {
test('it creates an Openable class implementing EventSource', function(assert) {
const EventSource = createEventSource();
const ReopenableEventSource = domEventSourceReopenable(EventSource);
assert.ok(ReopenableEventSource instanceof Function);
const source = new ReopenableEventSource(function() {});
const OpenableEventSource = domEventSourceOpenable(EventSource);
assert.ok(OpenableEventSource instanceof Function);
const source = new OpenableEventSource(function() {});
assert.ok(source instanceof EventSource);
});
test('it reopens the event source when reopen is called', function(assert) {
const callable = this.stub();
const EventSource = createEventSource();
const ReopenableEventSource = domEventSourceReopenable(EventSource);
const source = new ReopenableEventSource(callable);
const OpenableEventSource = domEventSourceOpenable(EventSource);
const source = new OpenableEventSource(callable);
assert.equal(source.readyState, 1);
// first automatic EventSource `open`
assert.ok(callable.calledOnce);
source.readyState = 3;
source.reopen();
source.open();
// still only called once as it hasn't completely closed yet
// therefore is just opened by resetting the readyState
assert.ok(callable.calledOnce);
assert.equal(source.readyState, 1);
// properly close the source
source.readyState = 2;
source.reopen();
// this time it is reopened via a recall of the callable
source.open();
// this time it is opened via a recall of the callable
assert.ok(callable.calledTwice);
});

0 comments on commit 09d6cfb

Please sign in to comment.