Skip to content

Commit

Permalink
revert: revert #7067 so we throw an error for invalid event types (#7719
Browse files Browse the repository at this point in the history
)

BREAKING CHANGE: Instead of logging an error message, invalid events will now trigger an `Error` which will terminate the call stack.
  • Loading branch information
alex-barstow authored and misteroneill committed Nov 23, 2022
1 parent c190b21 commit f99ace0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 40 deletions.
11 changes: 2 additions & 9 deletions src/js/mixins/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import * as Fn from '../utils/fn';
import * as Obj from '../utils/obj';
import EventTarget from '../event-target';
import DomData from '../utils/dom-data';
import log from '../utils/log';

const objName = (obj) => {
if (typeof obj.name === 'function') {
Expand Down Expand Up @@ -446,14 +445,8 @@ const EventedMixin = {
const type = event && typeof event !== 'string' ? event.type : event;

if (!isValidEventType(type)) {
const error = `Invalid event type for ${objName(this)}#trigger; ` +
'must be a non-empty string or object with a type key that has a non-empty value.';

if (event) {
(this.log || log).error(error);
} else {
throw new Error(error);
}
throw new Error(`Invalid event type for ${objName(this)}#trigger; ` +
'must be a non-empty string or object with a type key that has a non-empty value.');
}
return Events.trigger(this.eventBusEl_, event, hash);
}
Expand Down
41 changes: 10 additions & 31 deletions test/unit/mixins/evented.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-env qunit */
import sinon from 'sinon';
import evented from '../../../src/js/mixins/evented';
import log from '../../../src/js/utils/log';
import DomData from '../../../src/js/utils/dom-data';
import * as Dom from '../../../src/js/utils/dom';
import * as Obj from '../../../src/js/utils/obj';
Expand Down Expand Up @@ -67,46 +66,26 @@ QUnit.test('evented() with custom element', function(assert) {

QUnit.test('trigger() errors', function(assert) {
class Test {}

const tester = new Test();
const targeta = evented(tester);
const targeta = evented({});
const targetb = evented(new Test());
const targetc = evented(new Test());
const targetd = evented({});

tester.log = log.createLogger('tester');

sinon.stub(log, 'error');
sinon.stub(tester.log, 'error');

targetc.name_ = 'foo';

const createTest = (lg) => (target) => {
[targeta, targetb, targetc].forEach((target) => {
const objName = target.name_ || target.constructor.name || typeof target;
const triggerError = errors.trigger(objName);

assert.throws(() => target.trigger(), /^Error: Invalid event type/, 'threw an error when tried to trigger without an event');

target.trigger(' ');
target.trigger({});
target.trigger({type: ''});
target.trigger({type: ' '});

assert.ok(lg.error.called, 'error was called');
assert.equal(lg.error.callCount, 4, 'log.error called 4 times');
assert.ok(lg.error.calledWithMatch(new RegExp(`^Invalid event type for ${objName}#trigger`)), 'error called with expected message');
assert.throws(() => target.trigger(), triggerError, 'expected error');
assert.throws(() => target.trigger(' '), triggerError, 'expected error');
assert.throws(() => target.trigger({}), triggerError, 'expected error');
assert.throws(() => target.trigger({type: ''}), triggerError, 'expected error');
assert.throws(() => target.trigger({type: ' '}), triggerError, 'expected error');

delete target.eventBusEl_;

assert.throws(() => target.trigger({type: 'foo'}), new RegExp(`^Error: Invalid target for ${objName}#trigger`), 'expected error');

lg.error.reset();
};

createTest(targeta.log)(targeta);
[targetb, targetc, targetd].forEach(createTest(log));

targeta.log.error.restore();
log.error.restore();
assert.throws(() => target.trigger({type: 'foo'}), errors.target(objName, 'trigger'), 'expected error');
});
});

QUnit.test('on(), one(), and any() errors', function(assert) {
Expand Down

0 comments on commit f99ace0

Please sign in to comment.