Skip to content

Commit

Permalink
fix(scheduler): prevent unwanted clearInterval (#3226)
Browse files Browse the repository at this point in the history
* test(scheduler): add interval recycling tests

* fix(scheduler): prevent unwanted clearInterval

In AsyncAction, this.pending was assigned before the call to
recycleAsyncId. That prevented the interval from being re-used resulting
in the interval being cleared and re-created for each notification.

Closes #3042
  • Loading branch information
cartant authored and benlesh committed Jan 16, 2018
1 parent 275a9a3 commit d7cfb42
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
53 changes: 53 additions & 0 deletions spec/schedulers/AsapScheduler-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,59 @@ describe('Scheduler.asap', () => {
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 = (<any> sinon.stub(global, 'setInterval')).callThrough();
function dispatch(state: any): void {
state.index += 1;
if (state.index < 3) {
(<any> this).schedule(state, state.period);
}
}
const period = 50;
const state = { index: 0, period };
asap.schedule(dispatch, period, state);
expect(state).to.have.property('index', 0);
expect(stubSetInterval).to.have.property('callCount', 1);
fakeTimer.tick(period);
expect(state).to.have.property('index', 1);
expect(stubSetInterval).to.have.property('callCount', 1);
fakeTimer.tick(period);
expect(state).to.have.property('index', 2);
expect(stubSetInterval).to.have.property('callCount', 1);
stubSetInterval.restore();
sandbox.restore();
});

it('should not reuse the interval for recursively scheduled actions with a different 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 = (<any> sinon.stub(global, 'setInterval')).callThrough();
function dispatch(state: any): void {
state.index += 1;
state.period -= 1;
if (state.index < 3) {
(<any> this).schedule(state, state.period);
}
}
const period = 50;
const state = { index: 0, period };
asap.schedule(dispatch, period, state);
expect(state).to.have.property('index', 0);
expect(stubSetInterval).to.have.property('callCount', 1);
fakeTimer.tick(period);
expect(state).to.have.property('index', 1);
expect(stubSetInterval).to.have.property('callCount', 2);
fakeTimer.tick(period);
expect(state).to.have.property('index', 2);
expect(stubSetInterval).to.have.property('callCount', 3);
stubSetInterval.restore();
sandbox.restore();
});

it('should schedule an action to happen later', (done: MochaDone) => {
let actionHappened = false;
asap.schedule(() => {
Expand Down
8 changes: 4 additions & 4 deletions src/internal/scheduler/AsyncAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ export class AsyncAction<T> extends Action<T> {
// Always replace the current state with the new state.
this.state = state;

// Set the pending flag indicating that this action has been scheduled, or
// has recursively rescheduled itself.
this.pending = true;

const id = this.id;
const scheduler = this.scheduler;

Expand Down Expand Up @@ -61,6 +57,10 @@ export class AsyncAction<T> extends Action<T> {
this.id = this.recycleAsyncId(scheduler, id, delay);
}

// Set the pending flag indicating that this action has been scheduled, or
// has recursively rescheduled itself.
this.pending = true;

this.delay = delay;
// If this action has already an async Id, don't request a new one.
this.id = this.id || this.requestAsyncId(scheduler, this.id, delay);
Expand Down

0 comments on commit d7cfb42

Please sign in to comment.