From b02aa369cad4b8ce99639574db254237d68faeb5 Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Tue, 19 Sep 2017 13:24:46 -0500 Subject: [PATCH 1/8] rebase atom.commands.onDidFinish --- spec/command-registry-spec.js | 82 +++++++++++++++++++++++++++++++++-- src/command-registry.js | 23 +++++++--- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index a0ac86c08..eccac3de3 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -191,9 +191,11 @@ describe("CommandRegistry", () => { expect(calls).toEqual([]); }); - it("invokes callbacks registered with ::onWillDispatch and ::onDidDispatch", () => { + it("invokes callbacks registered with ::onWillDispatch and ::onDidDispatch and ::onDidFinish", () => { const sequence = []; + registry.onDidFinish(event => sequence.push(['onDidFinish', event])); + registry.onDidDispatch(event => sequence.push(['onDidDispatch', event])); registry.add('.grandchild', 'command', event => sequence.push(['listener', event])); @@ -206,9 +208,81 @@ describe("CommandRegistry", () => { expect(sequence[1][0]).toBe('listener'); expect(sequence[2][0]).toBe('onDidDispatch'); - expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1]).toBe(true); - expect(sequence[0][1].constructor).toBe(CustomEvent); - expect(sequence[0][1].target).toBe(grandchild); + waitsFor(() => sequence.length === 4), "onDidFinish never called"); + + runs(() => { + expect(sequence[3][0]).toBe 'onDidFinish' + + expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1] && sequence[2][1] === sequence[3][1]).toBe(true); + expect(sequence[0][1].constructor).toBe(CustomEvent); + expect(sequence[0][1].target).toBe(grandchild); + }); + }); + + it("invokes callbacks registered with ::onDidFinish on resolve", () => { + const sequence = []; + + registry.onDidFinish(event => { + sequence.push(['onDidFinish', event]); + }); + + registry.add('.grandchild', 'command', event => { + sequence.push(['listener', event]); + return new Promise(resolve => { + setTimeout(() => { + sequence.push(['resolve', event]); + resolve(); + }, 100); + }); + }); + + grandchild.dispatchEvent(new CustomEvent('command', {bubbles: true})); + advanceClock(100); + + waitsFor(() => sequence.length === 3, "onDidFinish never called for resolve"); + + runs(() => { + expect(sequence[0][0]).toBe('listener') + expect(sequence[1][0]).toBe('resolve') + expect(sequence[2][0]).toBe('onDidFinish') + + expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1]).toBe(true) + expect(sequence[0][1].constructor).toBe(CustomEvent) + expect(sequence[0][1].target).toBe(grandchild) + }); + }); + + it("invokes callbacks registered with ::onDidFinish on reject", () => { + const sequence = []; + + registry.onDidFinish(event => { + sequence.push(['onDidFinish', event]); + }); + + registry.add('.grandchild', 'command', event => { + sequence.push(['listener', event]); + return new Promise((_, reject) => { + setTimeout(() => { + sequence.push(['reject', event]); + reject(); + }, 100); + }); + }); + + grandchild.dispatchEvent(new CustomEvent('command', {bubbles: true})); + advanceClock(100); + + waitsFor(() => sequence.length === 3, "onDidFinish never called for reject"); + + runs(() => { + expect(sequence[0][0]).toBe('listener') + expect(sequence[1][0]).toBe('reject') + expect(sequence[2][0]).toBe('onDidFinish') + + expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1]).toBe(true) + expect(sequence[0][1].constructor).toBe(CustomEvent) + expect(sequence[0][1].target).toBe(grandchild) + }); }); }); diff --git a/src/command-registry.js b/src/command-registry.js index 9e6d8c2e1..e87c6a8d8 100644 --- a/src/command-registry.js +++ b/src/command-registry.js @@ -289,6 +289,14 @@ module.exports = class CommandRegistry { return this.emitter.on('did-dispatch', callback) } + // Public: Invoke the given callback after finishing a command event. + // + // * `callback` {Function} to be called after finishing each command + // * `event` The Event that was dispatched + onDidFinish (callback) { + return this.emitter.on('did-finish', callback) + } + getSnapshot () { const snapshot = {} for (const commandName in this.selectorBasedListenersByCommandName) { @@ -309,7 +317,7 @@ module.exports = class CommandRegistry { handleCommandEvent (event) { let propagationStopped = false let immediatePropagationStopped = false - let matched = false + let matched = [] let currentTarget = event.target const dispatchedEvent = new CustomEvent(event.type, { @@ -373,10 +381,6 @@ module.exports = class CommandRegistry { listeners = selectorBasedListeners.concat(listeners) } - if (listeners.length > 0) { - matched = true - } - // Call inline listeners first in reverse registration order, // and selector-based listeners by specificity and reverse // registration order. @@ -385,7 +389,7 @@ module.exports = class CommandRegistry { if (immediatePropagationStopped) { break } - listener.didDispatch.call(currentTarget, dispatchedEvent) + matched.push(listener.didDispatch.call(currentTarget, dispatchedEvent)) } if (currentTarget === window) { @@ -399,7 +403,12 @@ module.exports = class CommandRegistry { this.emitter.emit('did-dispatch', dispatchedEvent) - return matched + Promise.all(matched).then( + _ => this.emitter.emit('did-finish', dispatchedEvent), + _ => this.emitter.emit('did-finish', dispatchedEvent) + ) + + return matched.length > 0 } commandRegistered (commandName) { From f66ae074701b4b29d820c4ab158bea216fafdfa4 Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Tue, 19 Sep 2017 15:33:01 -0500 Subject: [PATCH 2/8] fix tests --- spec/command-registry-spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index eccac3de3..a038a4c94 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -208,10 +208,10 @@ describe("CommandRegistry", () => { expect(sequence[1][0]).toBe('listener'); expect(sequence[2][0]).toBe('onDidDispatch'); - waitsFor(() => sequence.length === 4), "onDidFinish never called"); + waitsFor(() => sequence.length === 4, "onDidFinish never called"); runs(() => { - expect(sequence[3][0]).toBe 'onDidFinish' + expect(sequence[3][0]).toBe('onDidFinish'); expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1] && sequence[2][1] === sequence[3][1]).toBe(true); expect(sequence[0][1].constructor).toBe(CustomEvent); From 03d16c4f5d111373a4a7275fd07bb096805c9950 Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Fri, 5 Jan 2018 14:53:50 -0600 Subject: [PATCH 3/8] return promise.all from dispatch --- spec/command-registry-spec.js | 84 +++++------------------------------ src/command-registry.js | 15 +------ 2 files changed, 13 insertions(+), 86 deletions(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index a038a4c94..ab8007040 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -191,11 +191,9 @@ describe("CommandRegistry", () => { expect(calls).toEqual([]); }); - it("invokes callbacks registered with ::onWillDispatch and ::onDidDispatch and ::onDidFinish", () => { + it("invokes callbacks registered with ::onWillDispatch and ::onDidDispatch", () => { const sequence = []; - registry.onDidFinish(event => sequence.push(['onDidFinish', event])); - registry.onDidDispatch(event => sequence.push(['onDidDispatch', event])); registry.add('.grandchild', 'command', event => sequence.push(['listener', event])); @@ -208,80 +206,22 @@ describe("CommandRegistry", () => { expect(sequence[1][0]).toBe('listener'); expect(sequence[2][0]).toBe('onDidDispatch'); - waitsFor(() => sequence.length === 4, "onDidFinish never called"); - - runs(() => { - expect(sequence[3][0]).toBe('onDidFinish'); - - expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1] && sequence[2][1] === sequence[3][1]).toBe(true); - expect(sequence[0][1].constructor).toBe(CustomEvent); - expect(sequence[0][1].target).toBe(grandchild); - }); + expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1]).toBe(true); + expect(sequence[0][1].constructor).toBe(CustomEvent); + expect(sequence[0][1].target).toBe(grandchild); }); - it("invokes callbacks registered with ::onDidFinish on resolve", () => { - const sequence = []; + it("returns a promise", () => { + const calls = []; + registry.add('.grandchild', 'command', () => 'grandchild'); + registry.add(child, 'command', () => 'child-inline'); + registry.add('.child', 'command', () => 'child'); + registry.add('.parent', 'command', () => 'parent'); - registry.onDidFinish(event => { - sequence.push(['onDidFinish', event]); - }); - - registry.add('.grandchild', 'command', event => { - sequence.push(['listener', event]); - return new Promise(resolve => { - setTimeout(() => { - sequence.push(['resolve', event]); - resolve(); - }, 100); - }); - }); - - grandchild.dispatchEvent(new CustomEvent('command', {bubbles: true})); - advanceClock(100); - - waitsFor(() => sequence.length === 3, "onDidFinish never called for resolve"); + waitsForPromise(() => grandchild.dispatchEvent(new CustomEvent('command', {bubbles: true})).then(args => { calls = args; })); runs(() => { - expect(sequence[0][0]).toBe('listener') - expect(sequence[1][0]).toBe('resolve') - expect(sequence[2][0]).toBe('onDidFinish') - - expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1]).toBe(true) - expect(sequence[0][1].constructor).toBe(CustomEvent) - expect(sequence[0][1].target).toBe(grandchild) - }); - }); - - it("invokes callbacks registered with ::onDidFinish on reject", () => { - const sequence = []; - - registry.onDidFinish(event => { - sequence.push(['onDidFinish', event]); - }); - - registry.add('.grandchild', 'command', event => { - sequence.push(['listener', event]); - return new Promise((_, reject) => { - setTimeout(() => { - sequence.push(['reject', event]); - reject(); - }, 100); - }); - }); - - grandchild.dispatchEvent(new CustomEvent('command', {bubbles: true})); - advanceClock(100); - - waitsFor(() => sequence.length === 3, "onDidFinish never called for reject"); - - runs(() => { - expect(sequence[0][0]).toBe('listener') - expect(sequence[1][0]).toBe('reject') - expect(sequence[2][0]).toBe('onDidFinish') - - expect(sequence[0][1] === sequence[1][1] && sequence[1][1] === sequence[2][1]).toBe(true) - expect(sequence[0][1].constructor).toBe(CustomEvent) - expect(sequence[0][1].target).toBe(grandchild) + expect(calls).toEqual(['grandchild', 'child-inline', 'child', 'parent']); }); }); }); diff --git a/src/command-registry.js b/src/command-registry.js index e87c6a8d8..e503691db 100644 --- a/src/command-registry.js +++ b/src/command-registry.js @@ -289,14 +289,6 @@ module.exports = class CommandRegistry { return this.emitter.on('did-dispatch', callback) } - // Public: Invoke the given callback after finishing a command event. - // - // * `callback` {Function} to be called after finishing each command - // * `event` The Event that was dispatched - onDidFinish (callback) { - return this.emitter.on('did-finish', callback) - } - getSnapshot () { const snapshot = {} for (const commandName in this.selectorBasedListenersByCommandName) { @@ -403,12 +395,7 @@ module.exports = class CommandRegistry { this.emitter.emit('did-dispatch', dispatchedEvent) - Promise.all(matched).then( - _ => this.emitter.emit('did-finish', dispatchedEvent), - _ => this.emitter.emit('did-finish', dispatchedEvent) - ) - - return matched.length > 0 + return (matched.length > 0 ? Promise.all(matched) : null) } commandRegistered (commandName) { From 327ee33facefd20fe120c38d33646ca052af282d Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Fri, 5 Jan 2018 15:25:13 -0600 Subject: [PATCH 4/8] move test --- spec/command-registry-spec.js | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index ab8007040..bc7d25dee 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -210,20 +210,6 @@ describe("CommandRegistry", () => { expect(sequence[0][1].constructor).toBe(CustomEvent); expect(sequence[0][1].target).toBe(grandchild); }); - - it("returns a promise", () => { - const calls = []; - registry.add('.grandchild', 'command', () => 'grandchild'); - registry.add(child, 'command', () => 'child-inline'); - registry.add('.child', 'command', () => 'child'); - registry.add('.parent', 'command', () => 'parent'); - - waitsForPromise(() => grandchild.dispatchEvent(new CustomEvent('command', {bubbles: true})).then(args => { calls = args; })); - - runs(() => { - expect(calls).toEqual(['grandchild', 'child-inline', 'child', 'parent']); - }); - }); }); describe("::add(selector, commandName, callback)", () => { @@ -371,12 +357,12 @@ describe("CommandRegistry", () => { expect(called).toBe(true); }); - it("returns a boolean indicating whether any listeners matched the command", () => { + it("returns a promise if any listeners matched the command", () => { registry.add('.grandchild', 'command', () => {}); - expect(registry.dispatch(grandchild, 'command')).toBe(true); - expect(registry.dispatch(grandchild, 'bogus')).toBe(false); - expect(registry.dispatch(parent, 'command')).toBe(false); + expect(registry.dispatch(grandchild, 'command').constructor.name).toBe("Promise"); + expect(registry.dispatch(grandchild, 'bogus')).toBe(null); + expect(registry.dispatch(parent, 'command')).toBe(null); }); }); From 2f7de33ff0ea8d95d0259ae0928f0f057b6a976c Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Mon, 8 Jan 2018 16:38:25 -0600 Subject: [PATCH 5/8] add return value tests --- spec/command-registry-spec.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index bc7d25dee..3bf279c26 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -1,5 +1,6 @@ const CommandRegistry = require('../src/command-registry'); const _ = require('underscore-plus'); +const {it, fit, ffit, fffit, beforeEach, afterEach} = require('./async-spec-helpers'); describe("CommandRegistry", () => { let registry, parent, child, grandchild; @@ -364,6 +365,33 @@ describe("CommandRegistry", () => { expect(registry.dispatch(grandchild, 'bogus')).toBe(null); expect(registry.dispatch(parent, 'command')).toBe(null); }); + + it("returns a promise that resolves when the listeners resolve", async () => { + registry.add('.grandchild', 'command', () => 1); + registry.add('.grandchild', 'command', () => Promise.resolve(2)); + registry.add('.grandchild', 'command', () => new Promise((resolve) => { + setTimeout(() => { resolve(3); }, 100); + })); + + const values = await registry.dispatch(grandchild, 'command'); + expect(values).toEqual([1, 2, 3]); + }); + + it("returns a promise that rejects when a listener is rejected", async () => { + registry.add('.grandchild', 'command', () => 1); + registry.add('.grandchild', 'command', () => Promise.resolve(2)); + registry.add('.grandchild', 'command', () => new Promise((resolve, reject) => { + setTimeout(() => { reject(3); }, 100); + })); + + let value; + try { + value = await registry.dispatch(grandchild, 'command'); + } catch (err) { + value = err; + } + expect(value).toBe(3); + }); }); describe("::getSnapshot and ::restoreSnapshot", () => From ec07003d39531d38de9f6adad60263bb062c8b8a Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Mon, 8 Jan 2018 17:14:49 -0600 Subject: [PATCH 6/8] fix timeout --- spec/command-registry-spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index 3bf279c26..82c6c6723 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -370,7 +370,7 @@ describe("CommandRegistry", () => { registry.add('.grandchild', 'command', () => 1); registry.add('.grandchild', 'command', () => Promise.resolve(2)); registry.add('.grandchild', 'command', () => new Promise((resolve) => { - setTimeout(() => { resolve(3); }, 100); + global.setTimeout(() => { resolve(3); }, 100); })); const values = await registry.dispatch(grandchild, 'command'); @@ -381,7 +381,7 @@ describe("CommandRegistry", () => { registry.add('.grandchild', 'command', () => 1); registry.add('.grandchild', 'command', () => Promise.resolve(2)); registry.add('.grandchild', 'command', () => new Promise((resolve, reject) => { - setTimeout(() => { reject(3); }, 100); + global.setTimeout(() => { reject(3); }, 100); })); let value; From aabbea6542f5de76b7f651a7bee1927923c9f88c Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Mon, 8 Jan 2018 19:41:34 -0600 Subject: [PATCH 7/8] jasmine.useRealClock() --- spec/command-registry-spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index 82c6c6723..f67103f41 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -367,10 +367,11 @@ describe("CommandRegistry", () => { }); it("returns a promise that resolves when the listeners resolve", async () => { + jasmine.useRealClock(); registry.add('.grandchild', 'command', () => 1); registry.add('.grandchild', 'command', () => Promise.resolve(2)); registry.add('.grandchild', 'command', () => new Promise((resolve) => { - global.setTimeout(() => { resolve(3); }, 100); + setTimeout(() => { resolve(3); }, 1); })); const values = await registry.dispatch(grandchild, 'command'); @@ -378,10 +379,11 @@ describe("CommandRegistry", () => { }); it("returns a promise that rejects when a listener is rejected", async () => { + jasmine.useRealClock(); registry.add('.grandchild', 'command', () => 1); registry.add('.grandchild', 'command', () => Promise.resolve(2)); registry.add('.grandchild', 'command', () => new Promise((resolve, reject) => { - global.setTimeout(() => { reject(3); }, 100); + setTimeout(() => { reject(3); }, 1); })); let value; From 54d011450f2a95551415e8bc3bfc87d2c43dc00a Mon Sep 17 00:00:00 2001 From: Tony Brix Date: Mon, 8 Jan 2018 20:02:34 -0600 Subject: [PATCH 8/8] listener calls are reversed --- spec/command-registry-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/command-registry-spec.js b/spec/command-registry-spec.js index f67103f41..03ef0cc34 100644 --- a/spec/command-registry-spec.js +++ b/spec/command-registry-spec.js @@ -375,7 +375,7 @@ describe("CommandRegistry", () => { })); const values = await registry.dispatch(grandchild, 'command'); - expect(values).toEqual([1, 2, 3]); + expect(values).toEqual([3, 2, 1]); }); it("returns a promise that rejects when a listener is rejected", async () => {