From c922964b65d4ab50b04990b57ba9fe2e9b9c231b Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Mon, 8 Jul 2019 20:57:16 -0700 Subject: [PATCH 1/5] Add alternative signatures for Model#start and Model#on that don't use var-args The var-args in the middle of parameter lists for Model#start and Model#on make it difficult to write TypeScript type definitions for them. These backwards-compatible changes add alternative signatures that don't use var-args. 1) Model#start now supports a new array format for input paths: model.start(outPath, inPath1, inPath2, ..., fn); // Old model.start(outPath, [inPath1, inPath2, ...], fn); // New 2) Model#on now checks if the path pattern is a pre-split array, and if so, it calls the listener with a ____Event instance and the wildcard-captured segments as an array: model.on('change', 'foo.**', (captures..., value, previous, passed) => {}); // Old model.on('change', ['foo', '**'], (changeEvent, captures) => {}); // New A ChangeEvent would look like {value, previous, passed}. --- lib/Model/events.js | 205 ++++++++++++++++++++++++++++++++++++++++---- lib/Model/fn.js | 26 +++++- lib/util.js | 4 + 3 files changed, 212 insertions(+), 23 deletions(-) diff --git a/lib/Model/events.js b/lib/Model/events.js index 5700d0d32..e5fe44805 100644 --- a/lib/Model/events.js +++ b/lib/Model/events.js @@ -1,5 +1,8 @@ +// @ts-check + var EventEmitter = require('events').EventEmitter; var util = require('../util'); +/** @type any */ var Model = require('./Model'); // These events are re-emitted as 'all' events, and they are queued up and @@ -79,6 +82,11 @@ Model.prototype.emit = function(type) { } if (Model.MUTATOR_EVENTS[type]) { if (this._silent) return this; + // `segments` is almost definitely an array of strings. + // + // A search for `.emit(` shows that `segments` is generated from either + // `Model#_splitPath` or `Model#_dereference`, both of which return an array + // of strings. var segments = arguments[1]; var eventArgs = arguments[2]; this._emit(type + 'Immediate', segments, eventArgs); @@ -106,13 +114,13 @@ Model.prototype.emit = function(type) { Model.prototype._on = EventEmitter.prototype.on; Model.prototype.addListener = Model.prototype.on = function(type, pattern, cb) { - var listener = eventListener(this, pattern, cb); + var listener = eventListener(this, type, pattern, cb); this._on(type, listener); return listener; }; Model.prototype.once = function(type, pattern, cb) { - var listener = eventListener(this, pattern, cb); + var listener = eventListener(this, type, pattern, cb); function g() { var matches = listener.apply(null, arguments); if (matches) this.removeListener(type, g); @@ -221,35 +229,83 @@ Model.prototype.removeContextListeners = function(value) { return this; }; -function eventListener(model, subpattern, cb) { +/** + * @param {Model} model + * @param {string} eventType + * @param {string | Model} subpattern + * @param {Function} cb + */ +function eventListener(model, eventType, subpattern, cb) { if (cb) { // For signatures: // model.on('change', 'example.subpath', callback) // model.at('example').on('change', 'subpath', callback) var pattern = model.path(subpattern); - return modelEventListener(pattern, cb, model._eventContext); + return modelEventListener(eventType, pattern, cb, model._eventContext); } var path = model.path(); - cb = arguments[1]; + cb = arguments[2]; // For signature: // model.at('example').on('change', callback) - if (path) return modelEventListener(path, cb, model._eventContext); + if (path) return modelEventListener(eventType, path, cb, model._eventContext); // For signature: // model.on('normalEvent', callback) return cb; } -function modelEventListener(pattern, cb, eventContext) { - var patternSegments = util.castSegments(pattern.split('.')); - var testFn = testPatternFn(pattern, patternSegments); - - function modelListener(segments, eventArgs) { - var captures = testFn(segments); - if (!captures) return; +/** + * Returns a function that can be passed to `EventEmitter#on`, with some + * additional properties used for `Model#removeAllListeners`. + * + * When the function is called, it checks if the event matches `patternArg`, and + * if there's a match, it calls `cb`. + * + * @param {string} eventType + * @param {string | string[]} patternArg + * @param {Function} cb + * @param {*} eventContext + * @return {ModelListenerFn & ModelListenerProps} + */ +function modelEventListener(eventType, patternArg, cb, eventContext) { + var pattern, patternSegments, useEventClasses; + if (Array.isArray(patternArg)) { + // If `patternArg` was provided as an array, then the `cb` listener will be + // invoked with an `____Event` object. + useEventClasses = true; + patternSegments = util.castSegments(patternArg); + if (patternSegments.length === 1 && patternSegments[0] === '**') { + pattern = '**'; + } + } else { + // Old-style: If `patternArg` was provided as a string, then the `cb` + // listener will invoked with a variable number of arguments. + useEventClasses = false; + pattern = patternArg; + patternSegments = util.castSegments(patternArg.split('.')); + } + var testFn = testPatternFn(patternSegments, pattern); + + /** @type {any} */ + var modelListener; + if (useEventClasses) { + var eventFactory = getEventFactory(eventType); + modelListener = function(segments, eventArgs) { + var captures = testFn(segments); + if (!captures) return; + + var event = eventFactory(eventArgs); + cb(event, captures); + return true; + }; + } else { + modelListener = function(segments, eventArgs) { + var captures = testFn(segments); + if (!captures) return; - var args = (captures.length) ? captures.concat(eventArgs) : eventArgs; - cb.apply(null, args); - return true; + var args = (captures.length) ? captures.concat(eventArgs) : eventArgs; + cb.apply(null, args); + return true; + }; } // Used in Model#removeAllListeners @@ -260,7 +316,112 @@ function modelEventListener(pattern, cb, eventContext) { return modelListener; } -function testPatternFn(pattern, patternSegments) { +/** @typedef { (segments: string[], eventArgs: any[]) => (boolean | undefined) } ModelListenerFn */ +/** @typedef { {pattern: string, patternSegments: Array, eventContext: any} } ModelListenerProps */ + +/** + * Returns a factory function that creates an `___Event` object based on an + * old-style `eventArgs` array. + * + * @param {string} eventType + * @return {(eventArgs: any[]) => ChangeEvent | InsertEvent | RemoveEvent | MoveEvent | LoadEvent | UnloadEvent} + */ +function getEventFactory(eventType) { + switch (eventType) { + case 'change': + return function(eventArgs) { + return new ChangeEvent(eventArgs); + }; + case 'insert': + return function(eventArgs) { + return new InsertEvent(eventArgs); + }; + case 'remove': + return function(eventArgs) { + return new RemoveEvent(eventArgs); + }; + case 'move': + return function(eventArgs) { + return new MoveEvent(eventArgs); + }; + case 'load': + return function(eventArgs) { + return new LoadEvent(eventArgs); + }; + case 'unload': + return function(eventArgs) { + return new UnloadEvent(eventArgs); + }; + case 'all': + return function(eventArgs) { + var concreteEventType = eventArgs[0]; // 'change', 'insert', etc. + var concreteEventFactory = getEventFactory(concreteEventType); + return concreteEventFactory(eventArgs.slice(1)); + }; + default: throw new Error('Unknown event: ' + eventType); + } +} + +// These constructors accept the `eventArgs` array format that Racer uses +// internally when calling `Model#emit`. +// +// Eventually, Racer should switch to passing these events around directly, +// but that will require updating all the places that parse the `eventArgs` +// array format, to extract things like `passed`. + +function ChangeEvent(eventArgs) { + this.value = eventArgs[0]; + this.previous = eventArgs[1]; + this.passed = eventArgs[2]; +} +ChangeEvent.prototype.type = 'change'; + +function InsertEvent(eventArgs) { + this.index = eventArgs[0]; + this.values = eventArgs[1]; + this.passed = eventArgs[2]; +} +InsertEvent.prototype.type = 'insert'; + +function RemoveEvent(eventArgs) { + this.index = eventArgs[0]; + this.removed = eventArgs[1]; + this.passed = eventArgs[2]; +} +RemoveEvent.prototype.type = 'remove'; + +function MoveEvent(eventArgs) { + this.from = eventArgs[0]; + this.to = eventArgs[1]; + this.howMany = eventArgs[2]; + this.passed = eventArgs[3]; +} +MoveEvent.prototype.type = 'move'; + +function LoadEvent(eventArgs) { + this.document = eventArgs[0]; + this.passed = eventArgs[1]; +} +LoadEvent.prototype.type = 'load'; + +function UnloadEvent(eventArgs) { + this.previousDocument = eventArgs[0]; + this.passed = eventArgs[1]; +} +UnloadEvent.prototype.type = 'unload'; + +/** + * Returns a function that tests an array of event segments against the + * `patternSegments`. (`pattern` only matters if it's exactly `'**'`.) + * + * @param {Array} patternSegments + * @param {string?} pattern + * @return {(segments: string[]) => (string[] | undefined)} A function to test + * an array of event segments. If the event segments match, an array of 0 or + * more segments captured by `*` / `**` is returned. If the event segments + * don't match, `undefined` is returned. + */ +function testPatternFn(patternSegments, pattern) { if (pattern === '**') { return function testPattern(segments) { return [segments.join('.')]; @@ -279,6 +440,7 @@ function testPatternFn(pattern, patternSegments) { // if it ends in a rest wildcard and each of the corresponding // segments are wildcards or equal. if (patternLen === segments.length || endingRest) { + /** @type string[] */ var captures = []; for (var i = 0; i < patternLen; i++) { var patternSegment = patternSegments[i]; @@ -298,15 +460,20 @@ function testPatternFn(pattern, patternSegments) { }; } +/** + * @param {Array} segments + */ function stripRestWildcard(segments) { // ['example', '**'] -> ['example']; return true var lastIndex = segments.length - 1; - if (segments[lastIndex] === '**') { + var lastSegment = segments[lastIndex]; + if (lastSegment === '**') { segments.pop(); return true; } // ['example', 'subpath**'] -> ['example', 'subpath']; return true - var match = /^([^\*]+)\*\*$/.exec(segments[lastIndex]); + if (typeof lastSegment !== 'string') return false; + var match = /^([^\*]+)\*\*$/.exec(lastSegment); if (!match) return false; segments[lastIndex] = match[1]; return true; diff --git a/lib/Model/fn.js b/lib/Model/fn.js index 45706d062..930fd6392 100644 --- a/lib/Model/fn.js +++ b/lib/Model/fn.js @@ -37,22 +37,40 @@ function parseStartArguments(model, args, hasPath) { } else { fns = last; } + // For `Model#start`, the first parameter is the output path. var path; if (hasPath) { path = model.path(args.shift()); } + // The second-to-last original argument could be an options object. + // If it's not an array and not path-like, then it's an options object. + last = args[args.length - 1]; var options; - if (!model.isPath(args[args.length - 1])) { + if (!Array.isArray(last) && !model.isPath(last)) { options = args.pop(); } - var i = args.length; + + // `args` is just the input paths at this point. + var inputs; + if (args.length === 1 && Array.isArray(args[0])) { + // Inputs provided as one array: + // model.start(outPath, [inPath1, inPath2], fn); + inputs = args[0]; + } else { + // Inputs provided as var-args: + // model.start(outPath, inPath1, inPath2, fn); + inputs = args; + } + + // Normalize each input into a string path. + var i = inputs.length; while (i--) { - args[i] = model.path(args[i]); + inputs[i] = model.path(inputs[i]); } return { name: name, path: path, - inputPaths: args, + inputPaths: inputs, fns: fns, options: options }; diff --git a/lib/util.js b/lib/util.js index bcb2c46bf..14d467efb 100644 --- a/lib/util.js +++ b/lib/util.js @@ -54,6 +54,10 @@ AsyncGroup.prototype.add = function() { }; }; +/** + * @param {Array} segments + * @return {Array} + */ function castSegments(segments) { // Cast number path segments from strings to numbers for (var i = segments.length; i--;) { From 552dcdfb84d02829342c358e694a58e3d676b601 Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Wed, 10 Jul 2019 14:36:37 -0700 Subject: [PATCH 2/5] Add tests for non-vararg #start an #on, fix a bug with non-vararg #on --- lib/Model/events.js | 7 +- test/Model/events.js | 159 ++++++++++++++++++++++++++++++++++++++----- test/Model/fn.js | 81 ++++++++++++++++++++++ 3 files changed, 228 insertions(+), 19 deletions(-) diff --git a/lib/Model/events.js b/lib/Model/events.js index e5fe44805..7411fdd2b 100644 --- a/lib/Model/events.js +++ b/lib/Model/events.js @@ -240,7 +240,12 @@ function eventListener(model, eventType, subpattern, cb) { // For signatures: // model.on('change', 'example.subpath', callback) // model.at('example').on('change', 'subpath', callback) - var pattern = model.path(subpattern); + var pattern; + if (Array.isArray(subpattern)) { + pattern = model._splitPath().concat(subpattern); + } else { + pattern = model.path(subpattern); + } return modelEventListener(eventType, pattern, cb, model._eventContext); } var path = model.path(); diff --git a/test/Model/events.js b/test/Model/events.js index aa1d1113c..35f4ec55a 100644 --- a/test/Model/events.js +++ b/test/Model/events.js @@ -65,45 +65,45 @@ describe('Model events', function() { describe('move', function() { it('can move an item from the end to the beginning of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4]); - this.remote.on('move', '**', function(captures, from, to) { - expect(from).to.equal(4); - expect(to).to.equal(0); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(4); + expect(event.to).to.equal(0); done(); }); this.local.move('array', 4, 0, 1); }); it('can swap the first two items in the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(captures, from, to) { - expect(from).to.equal(1); - expect(to).to.equal(0); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(1); + expect(event.to).to.equal(0); done(); }); this.local.move('array', 1, 0, 1, function() {}); }); it('can move an item from the begnning to the end of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(captures, from, to) { - expect(from).to.equal(0); - expect(to).to.equal(4); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(0); + expect(event.to).to.equal(4); done(); }); this.local.move('array', 0, 4, 1, function() {}); }); it('supports a negative destination index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(captures, from, to) { - expect(from).to.equal(0); - expect(to).to.equal(4); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(0); + expect(event.to).to.equal(4); done(); }); this.local.move('array', 0, -1, 1, function() {}); }); it('supports a negative source index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(captures, from, to) { - expect(from).to.equal(4); - expect(to).to.equal(2); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(4); + expect(event.to).to.equal(2); done(); }); this.local.move('array', -1, 2, 1, function() {}); @@ -111,9 +111,132 @@ describe('Model events', function() { it('can move several items mid-array, with an event for each', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); var events = 0; - this.remote.on('move', '**', function(captures, from, to) { - expect(from).to.equal(1); - expect(to).to.equal(4); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(1); + expect(event.to).to.equal(4); + if (++events === 2) { + done(); + } + }); + this.local.move('array', 1, 3, 2, function() {}); + }); + }); + }); +}); + +describe('Model events (structured)', function() { + describe('mutator events', function() { + it('calls earlier listeners in the order of mutations', function(done) { + var model = (new racer.Model()).at('_page'); + var expectedPaths = ['a', 'b', 'c']; + model.on('change', ['**'], function(_event, captures) { + expect(captures).to.eql([expectedPaths.shift()]); + if (!expectedPaths.length) { + done(); + } + }); + model.on('change', ['a'], function() { + model.set('b', 2); + }); + model.on('change', ['b'], function() { + model.set('c', 3); + }); + model.set('a', 1); + }); + it('calls later listeners in the order of mutations', function(done) { + var model = (new racer.Model()).at('_page'); + model.on('change', ['a'], function() { + model.set('b', 2); + }); + model.on('change', ['b'], function() { + model.set('c', 3); + }); + var expectedPaths = ['a', 'b', 'c']; + model.on('change', ['**'], function(_event, captures) { + expect(captures).to.eql([expectedPaths.shift()]); + if (!expectedPaths.length) { + done(); + } + }); + model.set('a', 1); + }); + }); + + describe('remote events', function() { + beforeEach(function(done) { + var backend = racer.createBackend(); + var local = this.local = backend.createModel().scope('colors.green'); + var remote = this.remote = backend.createModel().scope('colors.green'); + local.create(function(err) { + if (err) return done(err); + remote.subscribe(done); + }); + }); + + describe('set', function() { + it('can raise events registered on array indices', function(done) { + this.local.set('array', [0, 1, 2, 3, 4], function() {}); + this.remote.on('change', ['array', '0'], function(event) { + expect(event.value).to.equal(1); + expect(event.previous).to.equal(0); + done(); + }); + this.local.set('array.0', 1); + }); + }); + + describe('move', function() { + it('can move an item from the end to the beginning of the array', function(done) { + this.local.set('array', [0, 1, 2, 3, 4]); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(4); + expect(event.to).to.equal(0); + done(); + }); + this.local.move('array', 4, 0, 1); + }); + it('can swap the first two items in the array', function(done) { + this.local.set('array', [0, 1, 2, 3, 4], function() {}); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(1); + expect(event.to).to.equal(0); + done(); + }); + this.local.move('array', 1, 0, 1, function() {}); + }); + it('can move an item from the begnning to the end of the array', function(done) { + this.local.set('array', [0, 1, 2, 3, 4], function() {}); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(0); + expect(event.to).to.equal(4); + done(); + }); + this.local.move('array', 0, 4, 1, function() {}); + }); + it('supports a negative destination index of -1 (for last)', function(done) { + this.local.set('array', [0, 1, 2, 3, 4], function() {}); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(0); + expect(event.to).to.equal(4); + done(); + }); + this.local.move('array', 0, -1, 1, function() {}); + }); + it('supports a negative source index of -1 (for last)', function(done) { + this.local.set('array', [0, 1, 2, 3, 4], function() {}); + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(4); + expect(event.to).to.equal(2); + done(); + }); + this.local.move('array', -1, 2, 1, function() {}); + }); + it('can move several items mid-array, with an event for each', function(done) { + this.local.set('array', [0, 1, 2, 3, 4], function() {}); + var events = 0; + this.remote.on('move', ['**'], function(event) { + expect(event.from).to.equal(1); + expect(event.to).to.equal(4); if (++events === 2) { done(); } diff --git a/test/Model/fn.js b/test/Model/fn.js index fd0b68086..9d1706f64 100644 --- a/test/Model/fn.js +++ b/test/Model/fn.js @@ -136,6 +136,87 @@ describe('fn', function() { expect(model.get('_nums.sum')).to.equal(5); }); }); + describe('start (array inputs) and stop with getter', function() { + it('sets the output immediately on start', function() { + var model = new Model(); + model.fn('sum', function(a, b) { + return a + b; + }); + model.set('_nums.a', 2); + model.set('_nums.b', 4); + var value = model.start('_nums.sum', ['_nums.a', '_nums.b'], 'sum'); + expect(value).to.equal(6); + expect(model.get('_nums.sum')).to.equal(6); + }); + it('sets the output when an input changes', function() { + var model = new Model(); + model.fn('sum', function(a, b) { + return a + b; + }); + model.set('_nums.a', 2); + model.set('_nums.b', 4); + model.start('_nums.sum', ['_nums.a', '_nums.b'], 'sum'); + expect(model.get('_nums.sum')).to.equal(6); + model.set('_nums.a', 5); + expect(model.get('_nums.sum')).to.equal(9); + }); + it('sets the output when a parent of the input changes', function() { + var model = new Model(); + model.fn('sum', function(a, b) { + return a + b; + }); + model.set('_nums.in', { + a: 2, + b: 4 + }); + model.start('_nums.sum', ['_nums.in.a', '_nums.in.b'], 'sum'); + expect(model.get('_nums.sum')).to.equal(6); + model.set('_nums.in', { + a: 5, + b: 7 + }); + expect(model.get('_nums.sum')).to.equal(12); + }); + it('does not set the output when a sibling of the input changes', function() { + var model = new Model(); + var count = 0; + model.fn('sum', function(a, b) { + count++; + return a + b; + }); + model.set('_nums.in', { + a: 2, + b: 4 + }); + model.start('_nums.sum', ['_nums.in.a', '_nums.in.b'], 'sum'); + expect(model.get('_nums.sum')).to.equal(6); + expect(count).to.equal(1); + model.set('_nums.in.a', 3); + expect(model.get('_nums.sum')).to.equal(7); + expect(count).to.equal(2); + model.set('_nums.in.c', -1); + expect(model.get('_nums.sum')).to.equal(7); + expect(count).to.equal(2); + }); + it('can call stop without start', function() { + var model = new Model(); + model.stop('_nums.sum'); + }); + it('stops updating after calling stop', function() { + var model = new Model(); + model.fn('sum', function(a, b) { + return a + b; + }); + model.set('_nums.a', 2); + model.set('_nums.b', 4); + model.start('_nums.sum', ['_nums.a', '_nums.b'], 'sum'); + model.set('_nums.a', 1); + expect(model.get('_nums.sum')).to.equal(5); + model.stop('_nums.sum'); + model.set('_nums.a', 3); + expect(model.get('_nums.sum')).to.equal(5); + }); + }); describe('start with async option', function() { it('sets the output immediately on start', function() { var model = new Model(); From 8cf47b5cbc58db9e071e0a05322086309fbe304d Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Wed, 10 Jul 2019 17:37:19 -0700 Subject: [PATCH 3/5] Model#on: Add {useEventObjects: true} option, revert pathSegments changes --- lib/Model/events.js | 153 ++++++++++++++++++++++++++----------------- test/Model/events.js | 82 +++++++++++------------ 2 files changed, 134 insertions(+), 101 deletions(-) diff --git a/lib/Model/events.js b/lib/Model/events.js index 7411fdd2b..7dcab061f 100644 --- a/lib/Model/events.js +++ b/lib/Model/events.js @@ -112,15 +112,27 @@ Model.prototype.emit = function(type) { }; Model.prototype._on = EventEmitter.prototype.on; +/** + * @param {string} type + * @param {string} pattern + * @param {Function} cb + * @param {ModelOnOptions} [options] + */ Model.prototype.addListener = -Model.prototype.on = function(type, pattern, cb) { - var listener = eventListener(this, type, pattern, cb); +Model.prototype.on = function(type, pattern, cb, options) { + var listener = eventListener(this, type, pattern, cb, options); this._on(type, listener); return listener; }; -Model.prototype.once = function(type, pattern, cb) { - var listener = eventListener(this, type, pattern, cb); +/** + * @param {string} type + * @param {string} pattern + * @param {Function} cb + * @param {ModelOnOptions} [options] + */ +Model.prototype.once = function(type, pattern, cb, options) { + var listener = eventListener(this, type, pattern, cb, options); function g() { var matches = listener.apply(null, arguments); if (matches) this.removeListener(type, g); @@ -129,6 +141,13 @@ Model.prototype.once = function(type, pattern, cb) { return g; }; +/** + * @typedef {Object} ModelOnOptions + * @property {boolean} [useEventObjects] - If true, the listener is called with + * `cb(event: ___Event, captures: string[])`, instead of the legacy var-args + * style `cb(captures..., [eventType], eventArgs..., passed)`. + */ + Model.prototype._removeAllListeners = EventEmitter.prototype.removeAllListeners; Model.prototype.removeAllListeners = function(type, subpattern) { // If a pattern is specified without an event type, remove all model event @@ -234,30 +253,71 @@ Model.prototype.removeContextListeners = function(value) { * @param {string} eventType * @param {string | Model} subpattern * @param {Function} cb + * @param {ModelOnOptions} [options] */ -function eventListener(model, eventType, subpattern, cb) { +function eventListener(model, eventType, subpattern, cb, options) { + options = options || {}; if (cb) { - // For signatures: - // model.on('change', 'example.subpath', callback) + // For signatures with pattern: + // model.on('change', 'example.subpath.**', callback) // model.at('example').on('change', 'subpath', callback) - var pattern; - if (Array.isArray(subpattern)) { - pattern = model._splitPath().concat(subpattern); - } else { - pattern = model.path(subpattern); + if (options.useEventObjects) { + var pattern = model.path(subpattern); + return modelEventListener(eventType, pattern, cb, model._eventContext); + } else { // eslint-disable-line no-else-return + var pattern = model.path(subpattern); + return modelEventListenerLegacy(pattern, cb, model._eventContext); } - return modelEventListener(eventType, pattern, cb, model._eventContext); } + /** @type string */ var path = model.path(); cb = arguments[2]; - // For signature: + // For signature without explicit pattern: // model.at('example').on('change', callback) - if (path) return modelEventListener(eventType, path, cb, model._eventContext); + if (path) { + if (options.useEventObjects) { + return modelEventListener(eventType, path, cb, model._eventContext); + } else { // eslint-disable-line no-else-return + return modelEventListenerLegacy(path, cb, model._eventContext); + } + } // For signature: // model.on('normalEvent', callback) return cb; } +/** + * Legacy version of `modelEventListener` that calls `cb` with var-args + * `(captures..., [eventType], args..., passed)` instead of new-style + * `___Event` objects. + * + * @param {string} pattern + * @param {Function} cb + * @param {*} eventContext + * @return {ModelListenerFn & ModelListenerProps} + */ +function modelEventListenerLegacy(pattern, cb, eventContext) { + var patternSegments = util.castSegments(pattern.split('.')); + var testFn = testPatternFn(pattern, patternSegments); + + /** @type ModelListenerFn */ + function modelListener(segments, eventArgs) { + var captures = testFn(segments); + if (!captures) return; + + var args = (captures.length) ? captures.concat(eventArgs) : eventArgs; + cb.apply(null, args); + return true; + } + + // Used in Model#removeAllListeners + modelListener.pattern = pattern; + modelListener.patternSegments = patternSegments; + modelListener.eventContext = eventContext; + + return modelListener; +} + /** * Returns a function that can be passed to `EventEmitter#on`, with some * additional properties used for `Model#removeAllListeners`. @@ -266,51 +326,24 @@ function eventListener(model, eventType, subpattern, cb) { * if there's a match, it calls `cb`. * * @param {string} eventType - * @param {string | string[]} patternArg + * @param {string} pattern * @param {Function} cb * @param {*} eventContext * @return {ModelListenerFn & ModelListenerProps} */ -function modelEventListener(eventType, patternArg, cb, eventContext) { - var pattern, patternSegments, useEventClasses; - if (Array.isArray(patternArg)) { - // If `patternArg` was provided as an array, then the `cb` listener will be - // invoked with an `____Event` object. - useEventClasses = true; - patternSegments = util.castSegments(patternArg); - if (patternSegments.length === 1 && patternSegments[0] === '**') { - pattern = '**'; - } - } else { - // Old-style: If `patternArg` was provided as a string, then the `cb` - // listener will invoked with a variable number of arguments. - useEventClasses = false; - pattern = patternArg; - patternSegments = util.castSegments(patternArg.split('.')); - } - var testFn = testPatternFn(patternSegments, pattern); - - /** @type {any} */ - var modelListener; - if (useEventClasses) { - var eventFactory = getEventFactory(eventType); - modelListener = function(segments, eventArgs) { - var captures = testFn(segments); - if (!captures) return; - - var event = eventFactory(eventArgs); - cb(event, captures); - return true; - }; - } else { - modelListener = function(segments, eventArgs) { - var captures = testFn(segments); - if (!captures) return; - - var args = (captures.length) ? captures.concat(eventArgs) : eventArgs; - cb.apply(null, args); - return true; - }; +function modelEventListener(eventType, pattern, cb, eventContext) { + var patternSegments = util.castSegments(pattern.split('.')); + var testFn = testPatternFn(pattern, patternSegments); + + var eventFactory = getEventFactory(eventType); + /** @type ModelListenerFn */ + function modelListener(segments, eventArgs) { + var captures = testFn(segments); + if (!captures) return; + + var event = eventFactory(eventArgs); + cb(event, captures); + return true; } // Used in Model#removeAllListeners @@ -419,14 +452,14 @@ UnloadEvent.prototype.type = 'unload'; * Returns a function that tests an array of event segments against the * `patternSegments`. (`pattern` only matters if it's exactly `'**'`.) * - * @param {Array} patternSegments * @param {string?} pattern + * @param {Array} patternSegments * @return {(segments: string[]) => (string[] | undefined)} A function to test * an array of event segments. If the event segments match, an array of 0 or - * more segments captured by `*` / `**` is returned. If the event segments - * don't match, `undefined` is returned. + * more segments captured by `'*'` / `'**'` is returned, one per wildcard. If + * the event segments don't match, `undefined` is returned. */ -function testPatternFn(patternSegments, pattern) { +function testPatternFn(pattern, patternSegments) { if (pattern === '**') { return function testPattern(segments) { return [segments.join('.')]; diff --git a/test/Model/events.js b/test/Model/events.js index 35f4ec55a..8a2c46a38 100644 --- a/test/Model/events.js +++ b/test/Model/events.js @@ -65,45 +65,45 @@ describe('Model events', function() { describe('move', function() { it('can move an item from the end to the beginning of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4]); - this.remote.on('move', ['**'], function(event) { - expect(event.from).to.equal(4); - expect(event.to).to.equal(0); + this.remote.on('move', '**', function(captures, from, to) { + expect(from).to.equal(4); + expect(to).to.equal(0); done(); }); this.local.move('array', 4, 0, 1); }); it('can swap the first two items in the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { - expect(event.from).to.equal(1); - expect(event.to).to.equal(0); + this.remote.on('move', '**', function(captures, from, to) { + expect(from).to.equal(1); + expect(to).to.equal(0); done(); }); this.local.move('array', 1, 0, 1, function() {}); }); it('can move an item from the begnning to the end of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { - expect(event.from).to.equal(0); - expect(event.to).to.equal(4); + this.remote.on('move', '**', function(captures, from, to) { + expect(from).to.equal(0); + expect(to).to.equal(4); done(); }); this.local.move('array', 0, 4, 1, function() {}); }); it('supports a negative destination index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { - expect(event.from).to.equal(0); - expect(event.to).to.equal(4); + this.remote.on('move', '**', function(captures, from, to) { + expect(from).to.equal(0); + expect(to).to.equal(4); done(); }); this.local.move('array', 0, -1, 1, function() {}); }); it('supports a negative source index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { - expect(event.from).to.equal(4); - expect(event.to).to.equal(2); + this.remote.on('move', '**', function(captures, from, to) { + expect(from).to.equal(4); + expect(to).to.equal(2); done(); }); this.local.move('array', -1, 2, 1, function() {}); @@ -111,9 +111,9 @@ describe('Model events', function() { it('can move several items mid-array, with an event for each', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); var events = 0; - this.remote.on('move', ['**'], function(event) { - expect(event.from).to.equal(1); - expect(event.to).to.equal(4); + this.remote.on('move', '**', function(captures, from, to) { + expect(from).to.equal(1); + expect(to).to.equal(4); if (++events === 2) { done(); } @@ -124,40 +124,40 @@ describe('Model events', function() { }); }); -describe('Model events (structured)', function() { +describe('Model events with {useEventObjects: true}', function() { describe('mutator events', function() { it('calls earlier listeners in the order of mutations', function(done) { var model = (new racer.Model()).at('_page'); var expectedPaths = ['a', 'b', 'c']; - model.on('change', ['**'], function(_event, captures) { + model.on('change', '**', function(_event, captures) { expect(captures).to.eql([expectedPaths.shift()]); if (!expectedPaths.length) { done(); } - }); - model.on('change', ['a'], function() { + }, {useEventObjects: true}); + model.on('change', 'a', function() { model.set('b', 2); }); - model.on('change', ['b'], function() { + model.on('change', 'b', function() { model.set('c', 3); }); model.set('a', 1); }); it('calls later listeners in the order of mutations', function(done) { var model = (new racer.Model()).at('_page'); - model.on('change', ['a'], function() { + model.on('change', 'a', function() { model.set('b', 2); }); - model.on('change', ['b'], function() { + model.on('change', 'b', function() { model.set('c', 3); }); var expectedPaths = ['a', 'b', 'c']; - model.on('change', ['**'], function(_event, captures) { + model.on('change', '**', function(_event, captures) { expect(captures).to.eql([expectedPaths.shift()]); if (!expectedPaths.length) { done(); } - }); + }, {useEventObjects: true}); model.set('a', 1); }); }); @@ -176,11 +176,11 @@ describe('Model events (structured)', function() { describe('set', function() { it('can raise events registered on array indices', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('change', ['array', '0'], function(event) { + this.remote.on('change', 'array.0', function(event) { expect(event.value).to.equal(1); expect(event.previous).to.equal(0); done(); - }); + }, {useEventObjects: true}); this.local.set('array.0', 1); }); }); @@ -188,59 +188,59 @@ describe('Model events (structured)', function() { describe('move', function() { it('can move an item from the end to the beginning of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4]); - this.remote.on('move', ['**'], function(event) { + this.remote.on('move', '**', function(event) { expect(event.from).to.equal(4); expect(event.to).to.equal(0); done(); - }); + }, {useEventObjects: true}); this.local.move('array', 4, 0, 1); }); it('can swap the first two items in the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { + this.remote.on('move', '**', function(event) { expect(event.from).to.equal(1); expect(event.to).to.equal(0); done(); - }); + }, {useEventObjects: true}); this.local.move('array', 1, 0, 1, function() {}); }); it('can move an item from the begnning to the end of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { + this.remote.on('move', '**', function(event) { expect(event.from).to.equal(0); expect(event.to).to.equal(4); done(); - }); + }, {useEventObjects: true}); this.local.move('array', 0, 4, 1, function() {}); }); it('supports a negative destination index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { + this.remote.on('move', '**', function(event) { expect(event.from).to.equal(0); expect(event.to).to.equal(4); done(); - }); + }, {useEventObjects: true}); this.local.move('array', 0, -1, 1, function() {}); }); it('supports a negative source index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', ['**'], function(event) { + this.remote.on('move', '**', function(event) { expect(event.from).to.equal(4); expect(event.to).to.equal(2); done(); - }); + }, {useEventObjects: true}); this.local.move('array', -1, 2, 1, function() {}); }); it('can move several items mid-array, with an event for each', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); var events = 0; - this.remote.on('move', ['**'], function(event) { + this.remote.on('move', '**', function(event) { expect(event.from).to.equal(1); expect(event.to).to.equal(4); if (++events === 2) { done(); } - }); + }, {useEventObjects: true}); this.local.move('array', 1, 3, 2, function() {}); }); }); From d1dcca4390bad89a6a65faa19c80223301f660f9 Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Thu, 11 Jul 2019 11:36:08 -0700 Subject: [PATCH 4/5] Move Model#on options to before callback --- lib/Model/events.js | 62 +++++++++++++++++++++++++------------------- test/Model/events.js | 36 ++++++++++++------------- 2 files changed, 53 insertions(+), 45 deletions(-) diff --git a/lib/Model/events.js b/lib/Model/events.js index 7dcab061f..7db263ebc 100644 --- a/lib/Model/events.js +++ b/lib/Model/events.js @@ -112,27 +112,15 @@ Model.prototype.emit = function(type) { }; Model.prototype._on = EventEmitter.prototype.on; -/** - * @param {string} type - * @param {string} pattern - * @param {Function} cb - * @param {ModelOnOptions} [options] - */ Model.prototype.addListener = -Model.prototype.on = function(type, pattern, cb, options) { - var listener = eventListener(this, type, pattern, cb, options); +Model.prototype.on = function(type, pattern, options, cb) { + var listener = eventListener(this, type, pattern, options, cb); this._on(type, listener); return listener; }; -/** - * @param {string} type - * @param {string} pattern - * @param {Function} cb - * @param {ModelOnOptions} [options] - */ -Model.prototype.once = function(type, pattern, cb, options) { - var listener = eventListener(this, type, pattern, cb, options); +Model.prototype.once = function(type, pattern, options, cb) { + var listener = eventListener(this, type, pattern, options, cb); function g() { var matches = listener.apply(null, arguments); if (matches) this.removeListener(type, g); @@ -251,17 +239,38 @@ Model.prototype.removeContextListeners = function(value) { /** * @param {Model} model * @param {string} eventType - * @param {string | Model} subpattern - * @param {Function} cb - * @param {ModelOnOptions} [options] */ -function eventListener(model, eventType, subpattern, cb, options) { - options = options || {}; - if (cb) { +function eventListener(model, eventType, arg2, arg3, arg4) { + var subpattern, options, cb; + if (arg4) { + // on(eventType, path, options, cb) + subpattern = arg2; + options = arg3; + cb = arg4; + } else if (arg3) { + // on(eventType, path, cb) + // on(eventType, options, cb) + cb = arg3; + if (model.isPath(arg2)) { + subpattern = arg2; + } else { + options = arg2; + } + } else { // if (arg2) + // on(eventType, cb) + cb = arg2; + } + if (options) { + if (options.useEventObjects) { + var useEventObjects = true; + } + } + + if (subpattern) { // For signatures with pattern: // model.on('change', 'example.subpath.**', callback) // model.at('example').on('change', 'subpath', callback) - if (options.useEventObjects) { + if (useEventObjects) { var pattern = model.path(subpattern); return modelEventListener(eventType, pattern, cb, model._eventContext); } else { // eslint-disable-line no-else-return @@ -269,13 +278,12 @@ function eventListener(model, eventType, subpattern, cb, options) { return modelEventListenerLegacy(pattern, cb, model._eventContext); } } - /** @type string */ - var path = model.path(); - cb = arguments[2]; // For signature without explicit pattern: // model.at('example').on('change', callback) + /** @type string */ + var path = model.path(); if (path) { - if (options.useEventObjects) { + if (useEventObjects) { return modelEventListener(eventType, path, cb, model._eventContext); } else { // eslint-disable-line no-else-return return modelEventListenerLegacy(path, cb, model._eventContext); diff --git a/test/Model/events.js b/test/Model/events.js index 8a2c46a38..8d835a842 100644 --- a/test/Model/events.js +++ b/test/Model/events.js @@ -129,12 +129,12 @@ describe('Model events with {useEventObjects: true}', function() { it('calls earlier listeners in the order of mutations', function(done) { var model = (new racer.Model()).at('_page'); var expectedPaths = ['a', 'b', 'c']; - model.on('change', '**', function(_event, captures) { + model.on('change', '**', {useEventObjects: true}, function(_event, captures) { expect(captures).to.eql([expectedPaths.shift()]); if (!expectedPaths.length) { done(); } - }, {useEventObjects: true}); + }); model.on('change', 'a', function() { model.set('b', 2); }); @@ -152,12 +152,12 @@ describe('Model events with {useEventObjects: true}', function() { model.set('c', 3); }); var expectedPaths = ['a', 'b', 'c']; - model.on('change', '**', function(_event, captures) { + model.on('change', '**', {useEventObjects: true}, function(_event, captures) { expect(captures).to.eql([expectedPaths.shift()]); if (!expectedPaths.length) { done(); } - }, {useEventObjects: true}); + }); model.set('a', 1); }); }); @@ -176,11 +176,11 @@ describe('Model events with {useEventObjects: true}', function() { describe('set', function() { it('can raise events registered on array indices', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('change', 'array.0', function(event) { + this.remote.on('change', 'array.0', {useEventObjects: true}, function(event) { expect(event.value).to.equal(1); expect(event.previous).to.equal(0); done(); - }, {useEventObjects: true}); + }); this.local.set('array.0', 1); }); }); @@ -188,59 +188,59 @@ describe('Model events with {useEventObjects: true}', function() { describe('move', function() { it('can move an item from the end to the beginning of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4]); - this.remote.on('move', '**', function(event) { + this.remote.on('move', '**', {useEventObjects: true}, function(event) { expect(event.from).to.equal(4); expect(event.to).to.equal(0); done(); - }, {useEventObjects: true}); + }); this.local.move('array', 4, 0, 1); }); it('can swap the first two items in the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(event) { + this.remote.on('move', '**', {useEventObjects: true}, function(event) { expect(event.from).to.equal(1); expect(event.to).to.equal(0); done(); - }, {useEventObjects: true}); + }); this.local.move('array', 1, 0, 1, function() {}); }); it('can move an item from the begnning to the end of the array', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(event) { + this.remote.on('move', '**', {useEventObjects: true}, function(event) { expect(event.from).to.equal(0); expect(event.to).to.equal(4); done(); - }, {useEventObjects: true}); + }); this.local.move('array', 0, 4, 1, function() {}); }); it('supports a negative destination index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(event) { + this.remote.on('move', '**', {useEventObjects: true}, function(event) { expect(event.from).to.equal(0); expect(event.to).to.equal(4); done(); - }, {useEventObjects: true}); + }); this.local.move('array', 0, -1, 1, function() {}); }); it('supports a negative source index of -1 (for last)', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); - this.remote.on('move', '**', function(event) { + this.remote.on('move', '**', {useEventObjects: true}, function(event) { expect(event.from).to.equal(4); expect(event.to).to.equal(2); done(); - }, {useEventObjects: true}); + }); this.local.move('array', -1, 2, 1, function() {}); }); it('can move several items mid-array, with an event for each', function(done) { this.local.set('array', [0, 1, 2, 3, 4], function() {}); var events = 0; - this.remote.on('move', '**', function(event) { + this.remote.on('move', '**', {useEventObjects: true}, function(event) { expect(event.from).to.equal(1); expect(event.to).to.equal(4); if (++events === 2) { done(); } - }, {useEventObjects: true}); + }); this.local.move('array', 1, 3, 2, function() {}); }); }); From 055b8f3951ef35b27b3e412a2ec49185d4779cb2 Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Thu, 11 Jul 2019 11:40:30 -0700 Subject: [PATCH 5/5] Style tweaks --- lib/Model/events.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/Model/events.js b/lib/Model/events.js index 7db263ebc..d700264ab 100644 --- a/lib/Model/events.js +++ b/lib/Model/events.js @@ -270,24 +270,19 @@ function eventListener(model, eventType, arg2, arg3, arg4) { // For signatures with pattern: // model.on('change', 'example.subpath.**', callback) // model.at('example').on('change', 'subpath', callback) - if (useEventObjects) { - var pattern = model.path(subpattern); - return modelEventListener(eventType, pattern, cb, model._eventContext); - } else { // eslint-disable-line no-else-return - var pattern = model.path(subpattern); - return modelEventListenerLegacy(pattern, cb, model._eventContext); - } + var pattern = model.path(subpattern); + return (useEventObjects) ? + modelEventListener(eventType, pattern, cb, model._eventContext) : + modelEventListenerLegacy(pattern, cb, model._eventContext); } // For signature without explicit pattern: // model.at('example').on('change', callback) /** @type string */ var path = model.path(); if (path) { - if (useEventObjects) { - return modelEventListener(eventType, path, cb, model._eventContext); - } else { // eslint-disable-line no-else-return - return modelEventListenerLegacy(path, cb, model._eventContext); - } + return (useEventObjects) ? + modelEventListener(eventType, path, cb, model._eventContext) : + modelEventListenerLegacy(path, cb, model._eventContext); } // For signature: // model.on('normalEvent', callback)