diff --git a/.prettierrc.json b/.prettierrc.json new file mode 100644 index 0000000000..f407755f5b --- /dev/null +++ b/.prettierrc.json @@ -0,0 +1,13 @@ +{ + "trailingComma": "es5", + "singleQuote": true, + "printWidth": 140, + "overrides": [ + { + "files": ["spec/**/*.ts", "spec-dtslint/**/*.ts"], + "options": { + "requirePragma": true + } + } + ] +} diff --git a/spec/schedulers/AnimationFrameScheduler-spec.ts b/spec/schedulers/AnimationFrameScheduler-spec.ts index 8e7b48c486..ab973d45ee 100644 --- a/spec/schedulers/AnimationFrameScheduler-spec.ts +++ b/spec/schedulers/AnimationFrameScheduler-spec.ts @@ -1,11 +1,27 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; import { animationFrameScheduler, Subscription } from 'rxjs'; +import { TestScheduler } from 'rxjs/testing'; +import { observableMatcher } from 'spec/helpers/observableMatcher'; const animationFrame = animationFrameScheduler; +declare const globalThis: NodeJS.Global | Window; + /** @test {Scheduler} */ describe('Scheduler.animationFrame', () => { + let testScheduler: TestScheduler; + + const ROOT = (typeof globalThis !== 'undefined' && globalThis) + || (typeof global !== 'undefined' && global) + || (typeof window !== 'undefined' && window) + || (typeof self !== 'undefined' && self); + + beforeEach(() => { + testScheduler = new TestScheduler(observableMatcher); + }); + + it('should exist', () => { expect(animationFrame).exist; }); @@ -25,20 +41,34 @@ describe('Scheduler.animationFrame', () => { sandbox.restore(); }); - it('should cancel animationFrame actions when unsubscribed', () => { - let actionHappened = false; - const sandbox = sinon.sandbox.create(); - const fakeTimer = sandbox.useFakeTimers(); - animationFrame.schedule(() => { - actionHappened = true; - }, 50).unsubscribe(); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - sandbox.restore(); - }); + // This test not supported in 6.x (ported from cherry-pick). + + // it('should cancel animationFrame actions when delay > 0', () => { + // testScheduler.run(({ animate, cold, expectObservable, flush }) => { + // const requestSpy = sinon.spy(ROOT, 'requestAnimationFrame'); + // const setSpy = sinon.spy(ROOT, 'setInterval'); + // const clearSpy = sinon.spy(ROOT, 'clearInterval'); + + // animate(' ----------x--'); + // const a = cold(' a '); + // const ta = 4; + // const subs = ' ^-! '; + // const expected = '-------------'; + + // const result = merge( + // a.pipe(delay(ta, animationFrame)) + // ); + // expectObservable(result, subs).toBe(expected); + + // flush(); + // expect(requestSpy).to.have.not.been.called; + // expect(setSpy).to.have.been.calledOnce; + // expect(clearSpy).to.have.been.calledOnce; + // requestSpy.restore(); + // setSpy.restore(); + // clearSpy.restore(); + // }); + // }); it('should schedule an action to happen later', (done: MochaDone) => { let actionHappened = false; @@ -115,4 +145,104 @@ describe('Scheduler.animationFrame', () => { firstSubscription.unsubscribe(); } }); + + it('should not execute rescheduled actions when flushing', (done) => { + let flushCount = 0; + let scheduledIndices: number[] = []; + + let originalFlush = animationFrame.flush; + animationFrame.flush = (...args) => { + ++flushCount; + originalFlush.apply(animationFrame, args); + if (flushCount === 2) { + animationFrame.flush = originalFlush; + try { + expect(scheduledIndices).to.deep.equal([0, 1]); + done(); + } catch (error) { + done(error); + } + } + }; + + animationFrame.schedule(function (index) { + if (flushCount < 2) { + this.schedule(index! + 1); + scheduledIndices.push(index! + 1); + } + }, 0, 0); + scheduledIndices.push(0); + }); + + it('should execute actions scheduled when flushing in a subsequent flush', (done) => { + const sandbox = sinon.createSandbox(); + const stubFlush = (sandbox.stub(animationFrameScheduler, 'flush')).callThrough(); + + let a: Subscription; + let b: Subscription; + let c: Subscription; + + a = animationFrameScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(1); + c = animationFrameScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(2); + sandbox.restore(); + done(); + }); + }); + b = animationFrameScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(1); + }); + }); + + it('should execute actions scheduled when flushing in a subsequent flush when some actions are unsubscribed', (done) => { + const sandbox = sinon.createSandbox(); + const stubFlush = (sandbox.stub(animationFrameScheduler, 'flush')).callThrough(); + + let a: Subscription; + let b: Subscription; + let c: Subscription; + + a = animationFrameScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(1); + c = animationFrameScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(2); + sandbox.restore(); + done(); + }); + b.unsubscribe(); + }); + b = animationFrameScheduler.schedule(() => { + done(new Error('Unexpected execution of b')); + }); + }); + + if ('cancelAnimationFrame' in ROOT) { + it('should properly cancel an unnecessary flush', (done) => { + const sandbox = sinon.createSandbox(); + const cancelAnimationFrameStub = sandbox.stub(ROOT, 'cancelAnimationFrame').callThrough(); + + let a: Subscription; + let b: Subscription; + let c: Subscription; + + a = animationFrameScheduler.schedule(() => { + expect(animationFrameScheduler.actions).to.have.length(1); + c = animationFrameScheduler.schedule(() => { + done(new Error('Unexpected execution of c')); + }); + expect(animationFrameScheduler.actions).to.have.length(2); + // What we're testing here is that the unsubscription of action c effects + // the cancellation of the animation frame in a scenario in which the + // actions queue is not empty - it contains action b. + c.unsubscribe(); + expect(animationFrameScheduler.actions).to.have.length(1); + expect(cancelAnimationFrameStub).to.have.callCount(1); + }); + b = animationFrameScheduler.schedule(() => { + sandbox.restore(); + done(); + }); + }); + } }); diff --git a/spec/schedulers/AsapScheduler-spec.ts b/spec/schedulers/AsapScheduler-spec.ts index be1a767035..357bf5513d 100644 --- a/spec/schedulers/AsapScheduler-spec.ts +++ b/spec/schedulers/AsapScheduler-spec.ts @@ -1,11 +1,28 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { asapScheduler, Subscription, SchedulerAction } from 'rxjs'; +import { asapScheduler, Subscription, SchedulerAction, merge } from 'rxjs'; +import { delay } from 'rxjs/operators'; +import { Immediate } from 'rxjs/internal/util/Immediate'; +import { TestScheduler } from 'rxjs/testing'; +import { observableMatcher } from 'spec/helpers/observableMatcher'; + +declare const globalThis: NodeJS.Global | Window; const asap = asapScheduler; /** @test {Scheduler} */ describe('Scheduler.asap', () => { + let testScheduler: TestScheduler; + + const ROOT = (typeof globalThis !== 'undefined' && globalThis) + || (typeof global !== 'undefined' && global) + || (typeof window !== 'undefined' && window) + || (typeof self !== 'undefined' && self); + + beforeEach(() => { + testScheduler = new TestScheduler(observableMatcher); + }); + it('should exist', () => { expect(asap).exist; }); @@ -26,25 +43,35 @@ describe('Scheduler.asap', () => { }); it('should cancel asap actions when delay > 0', () => { - let actionHappened = false; - const sandbox = sinon.sandbox.create(); - const fakeTimer = sandbox.useFakeTimers(); - asap.schedule(() => { - actionHappened = true; - }, 50).unsubscribe(); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - sandbox.restore(); + testScheduler.run(({ cold, expectObservable, flush }) => { + const sandbox = sinon.createSandbox(); + const setImmediateSpy = sandbox.spy(Immediate, 'setImmediate'); + const setSpy = sandbox.spy(ROOT, 'setInterval'); + const clearSpy = sandbox.spy(ROOT, 'clearInterval'); + + const a = cold(' a '); + const ta = 4; + const subs = ' ^-! '; + const expected = '-------------'; + + const result = merge( + a.pipe(delay(ta, asap)) + ); + expectObservable(result, subs).toBe(expected); + + flush(); + expect(setImmediateSpy).to.have.not.been.called; + expect(setSpy).to.have.been.calledOnce; + expect(clearSpy).to.have.been.calledOnce; + sandbox.restore(); + }); }); it('should reuse the interval for recursively scheduled actions with the same delay', () => { const sandbox = sinon.sandbox.create(); const fakeTimer = sandbox.useFakeTimers(); // callThrough is missing from the declarations installed by the typings tool in stable - const stubSetInterval = ( sinon.stub(global, 'setInterval')).callThrough(); + const stubSetInterval = ( sandbox.stub(ROOT, 'setInterval')).callThrough(); const period = 50; const state = { index: 0, period }; type State = typeof state; @@ -63,7 +90,6 @@ describe('Scheduler.asap', () => { fakeTimer.tick(period); expect(state).to.have.property('index', 2); expect(stubSetInterval).to.have.property('callCount', 1); - stubSetInterval.restore(); sandbox.restore(); }); @@ -71,7 +97,7 @@ describe('Scheduler.asap', () => { const sandbox = sinon.sandbox.create(); const fakeTimer = sandbox.useFakeTimers(); // callThrough is missing from the declarations installed by the typings tool in stable - const stubSetInterval = ( sinon.stub(global, 'setInterval')).callThrough(); + const stubSetInterval = ( sandbox.stub(ROOT, 'setInterval')).callThrough(); const period = 50; const state = { index: 0, period }; type State = typeof state; @@ -91,7 +117,6 @@ describe('Scheduler.asap', () => { fakeTimer.tick(period); expect(state).to.have.property('index', 2); expect(stubSetInterval).to.have.property('callCount', 3); - stubSetInterval.restore(); sandbox.restore(); }); @@ -170,4 +195,102 @@ describe('Scheduler.asap', () => { firstSubscription.unsubscribe(); } }); + + it('should not execute rescheduled actions when flushing', (done) => { + let flushCount = 0; + let scheduledIndices: number[] = []; + + let originalFlush = asap.flush; + asap.flush = (...args) => { + ++flushCount; + originalFlush.apply(asap, args); + if (flushCount === 2) { + asap.flush = originalFlush; + try { + expect(scheduledIndices).to.deep.equal([0, 1]); + done(); + } catch (error) { + done(error); + } + } + }; + + asap.schedule(function (index) { + if (flushCount < 2) { + this.schedule(index! + 1); + scheduledIndices.push(index! + 1); + } + }, 0, 0); + scheduledIndices.push(0); + }); + + it('should execute actions scheduled when flushing in a subsequent flush', (done) => { + const sandbox = sinon.createSandbox(); + const stubFlush = (sandbox.stub(asapScheduler, 'flush')).callThrough(); + + let a: Subscription; + let b: Subscription; + let c: Subscription; + + a = asapScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(1); + c = asapScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(2); + sandbox.restore(); + done(); + }); + }); + b = asapScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(1); + }); + }); + + it('should execute actions scheduled when flushing in a subsequent flush when some actions are unsubscribed', (done) => { + const sandbox = sinon.createSandbox(); + const stubFlush = (sandbox.stub(asapScheduler, 'flush')).callThrough(); + + let a: Subscription; + let b: Subscription; + let c: Subscription; + + a = asapScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(1); + c = asapScheduler.schedule(() => { + expect(stubFlush).to.have.callCount(2); + sandbox.restore(); + done(); + }); + b.unsubscribe(); + }); + b = asapScheduler.schedule(() => { + done(new Error('Unexpected execution of b')); + }); + }); + + it('should properly cancel an unnecessary flush', (done) => { + const sandbox = sinon.createSandbox(); + const clearImmediateStub = sandbox.stub(ROOT, 'clearImmediate').callThrough(); + + let a: Subscription; + let b: Subscription; + let c: Subscription; + + a = asapScheduler.schedule(() => { + expect(asapScheduler.actions).to.have.length(1); + c = asapScheduler.schedule(() => { + done(new Error('Unexpected execution of c')); + }); + expect(asapScheduler.actions).to.have.length(2); + // What we're testing here is that the unsubscription of action c effects + // the cancellation of the microtask in a scenario in which the actions + // queue is not empty - it contains action b. + c.unsubscribe(); + expect(asapScheduler.actions).to.have.length(1); + expect(clearImmediateStub).to.have.callCount(1); + }); + b = asapScheduler.schedule(() => { + sandbox.restore(); + done(); + }); + }); }); diff --git a/src/internal/scheduler/AnimationFrameAction.ts b/src/internal/scheduler/AnimationFrameAction.ts index e9ea64fac6..dbe29b5d58 100644 --- a/src/internal/scheduler/AnimationFrameAction.ts +++ b/src/internal/scheduler/AnimationFrameAction.ts @@ -8,9 +8,7 @@ import { SchedulerAction } from '../types'; * @extends {Ignored} */ export class AnimationFrameAction extends AsyncAction { - - constructor(protected scheduler: AnimationFrameScheduler, - protected work: (this: SchedulerAction, state?: T) => void) { + constructor(protected scheduler: AnimationFrameScheduler, protected work: (this: SchedulerAction, state?: T) => void) { super(scheduler, work); } @@ -24,8 +22,7 @@ export class AnimationFrameAction extends AsyncAction { // If an animation frame has already been requested, don't request another // one. If an animation frame hasn't been requested yet, request one. Return // the current animation frame request id. - return scheduler.scheduled || (scheduler.scheduled = requestAnimationFrame( - () => scheduler.flush(null))); + return scheduler.scheduled || (scheduler.scheduled = requestAnimationFrame(() => scheduler.flush(null))); } protected recycleAsyncId(scheduler: AnimationFrameScheduler, id?: any, delay: number = 0): any { // If delay exists and is greater than 0, or if the delay is null (the @@ -34,10 +31,10 @@ export class AnimationFrameAction extends AsyncAction { if ((delay !== null && delay > 0) || (delay === null && this.delay > 0)) { return super.recycleAsyncId(scheduler, id, delay); } - // If the scheduler queue is empty, cancel the requested animation frame and - // set the scheduled flag to undefined so the next AnimationFrameAction will - // request its own. - if (scheduler.actions.length === 0) { + // If the scheduler queue has no remaining actions with the same async id, + // cancel the requested animation frame and set the scheduled flag to + // undefined so the next AnimationFrameAction will request its own. + if (!scheduler.actions.some((action) => action.id === id)) { cancelAnimationFrame(id); scheduler.scheduled = undefined; } diff --git a/src/internal/scheduler/AnimationFrameScheduler.ts b/src/internal/scheduler/AnimationFrameScheduler.ts index c550429f17..1ee22c5a18 100644 --- a/src/internal/scheduler/AnimationFrameScheduler.ts +++ b/src/internal/scheduler/AnimationFrameScheduler.ts @@ -3,26 +3,33 @@ import { AsyncScheduler } from './AsyncScheduler'; export class AnimationFrameScheduler extends AsyncScheduler { public flush(action?: AsyncAction): void { - this.active = true; + // The async id that effects a call to flush is stored in `scheduled`. + // Before executing an action, it's necessary to check the action's async + // id to determine whether it's supposed to be executed in the current + // flush. + // Previous implementations of this method used a count to determine this, + // but that was unsound, as actions that are unsubscribed - i.e. cancelled - + // are removed from the actions array and that can shift actions that are + // scheduled to be executed in a subsequent flush into positions at which + // they are executed within the current flush. + const flushId = this.scheduled; this.scheduled = undefined; - const {actions} = this; + const { actions } = this; let error: any; - let index: number = -1; - let count: number = actions.length; - action = action || actions.shift(); + action = action || actions.shift()!; do { - if (error = action.execute(action.state, action.delay)) { + if ((error = action.execute(action.state, action.delay))) { break; } - } while (++index < count && (action = actions.shift())); + } while ((action = actions[0]) && action.id === flushId && actions.shift()); this.active = false; if (error) { - while (++index < count && (action = actions.shift())) { + while ((action = actions[0]) && action.id === flushId && actions.shift()) { action.unsubscribe(); } throw error; diff --git a/src/internal/scheduler/AsapAction.ts b/src/internal/scheduler/AsapAction.ts index 1fe1622db9..5fed8f91dc 100644 --- a/src/internal/scheduler/AsapAction.ts +++ b/src/internal/scheduler/AsapAction.ts @@ -8,9 +8,7 @@ import { SchedulerAction } from '../types'; * @extends {Ignored} */ export class AsapAction extends AsyncAction { - - constructor(protected scheduler: AsapScheduler, - protected work: (this: SchedulerAction, state?: T) => void) { + constructor(protected scheduler: AsapScheduler, protected work: (this: SchedulerAction, state?: T) => void) { super(scheduler, work); } @@ -24,9 +22,7 @@ export class AsapAction extends AsyncAction { // If a microtask has already been scheduled, don't schedule another // one. If a microtask hasn't been scheduled yet, schedule one now. Return // the current scheduled microtask id. - return scheduler.scheduled || (scheduler.scheduled = Immediate.setImmediate( - scheduler.flush.bind(scheduler, null) - )); + return scheduler.scheduled || (scheduler.scheduled = Immediate.setImmediate(scheduler.flush.bind(scheduler, null))); } protected recycleAsyncId(scheduler: AsapScheduler, id?: any, delay: number = 0): any { // If delay exists and is greater than 0, or if the delay is null (the @@ -35,10 +31,10 @@ export class AsapAction extends AsyncAction { if ((delay !== null && delay > 0) || (delay === null && this.delay > 0)) { return super.recycleAsyncId(scheduler, id, delay); } - // If the scheduler queue is empty, cancel the requested microtask and - // set the scheduled flag to undefined so the next AsapAction will schedule - // its own. - if (scheduler.actions.length === 0) { + // If the scheduler queue has no remaining actions with the same async id, + // cancel the requested microtask and set the scheduled flag to undefined + // so the next AsapAction will request its own. + if (!scheduler.actions.some((action) => action.id === id)) { Immediate.clearImmediate(id); scheduler.scheduled = undefined; } diff --git a/src/internal/scheduler/AsapScheduler.ts b/src/internal/scheduler/AsapScheduler.ts index 659aa5823c..44ac78708a 100644 --- a/src/internal/scheduler/AsapScheduler.ts +++ b/src/internal/scheduler/AsapScheduler.ts @@ -3,26 +3,33 @@ import { AsyncScheduler } from './AsyncScheduler'; export class AsapScheduler extends AsyncScheduler { public flush(action?: AsyncAction): void { - this.active = true; + // The async id that effects a call to flush is stored in `scheduled`. + // Before executing an action, it's necessary to check the action's async + // id to determine whether it's supposed to be executed in the current + // flush. + // Previous implementations of this method used a count to determine this, + // but that was unsound, as actions that are unsubscribed - i.e. cancelled - + // are removed from the actions array and that can shift actions that are + // scheduled to be executed in a subsequent flush into positions at which + // they are executed within the current flush. + const flushId = this.scheduled; this.scheduled = undefined; - const {actions} = this; + const { actions } = this; let error: any; - let index: number = -1; - let count: number = actions.length; - action = action || actions.shift(); + action = action || actions.shift()!; do { - if (error = action.execute(action.state, action.delay)) { + if ((error = action.execute(action.state, action.delay))) { break; } - } while (++index < count && (action = actions.shift())); + } while ((action = actions[0]) && action.id === flushId && actions.shift()); this.active = false; if (error) { - while (++index < count && (action = actions.shift())) { + while ((action = actions[0]) && action.id === flushId && actions.shift()) { action.unsubscribe(); } throw error;