Skip to content

Commit

Permalink
fix(animationFrameScheduler): some tasks are never flushed and someti…
Browse files Browse the repository at this point in the history
…mes it breaks completely (#7444)

* fix(tests): Minor fix sandbox fake timers

Sandbox was not cleaning up correctly and making fail the test that I added.

* fix(animationFrameSchduler): Add failing tests

* fix(animationFrameScheduler): Fix the scenarios in added tests

* chore(animationFrameScheduler): minor code improvement

* chore(animationFrameScheduler): minor test code improvement

Make test assert order more clear
  • Loading branch information
pmoleri authored Feb 18, 2025
1 parent 4a2d0d2 commit 8bbfa4e
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 5 deletions.
46 changes: 46 additions & 0 deletions spec/schedulers/AnimationFrameScheduler-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,52 @@ describe('Scheduler.animationFrame', () => {
}
});

it('should schedule next frame actions from a delayed one', (done) => {
animationFrame.schedule(() => {
animationFrame.schedule(() => { done(); });
}, 1);
});

it('should schedule 2 actions for a subsequent frame', (done) => {
let runFirst = false;
animationFrame.schedule(() => {
animationFrame.schedule(() => { runFirst = true; });
animationFrame.schedule(() => {
if (runFirst) {
done();
} else {
done(new Error('First action did not run'));
}
});
});
});

it('should handle delayed action without affecting next frame actions', (done) => {
let runDelayed = false;
let runFirst = false;
animationFrame.schedule(() => {
animationFrame.schedule(() => {
if (!runDelayed) {
done(new Error('Delayed action did not run'));
return;
}
runFirst = true;
});
animationFrame.schedule(() => {
if (!runFirst) {
done(new Error('First action did not run'));
} else {
done();
}
});

// This action will execute before the next frame because the delay is less than the one of the frame
animationFrame.schedule(() => {
runDelayed = true;
}, 1);
});
});

it('should not execute rescheduled actions when flushing', (done) => {
let flushCount = 0;
let scheduledIndices: number[] = [];
Expand Down
10 changes: 8 additions & 2 deletions spec/schedulers/AsapScheduler-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Scheduler.asap', () => {
const sandbox = sinon.createSandbox();
const fakeTimer = sandbox.useFakeTimers();
// callThrough is missing from the declarations installed by the typings tool in stable
const stubSetInterval = (<any> sandbox.stub(global, 'setInterval')).callThrough();
const stubSetInterval = (<any> sandbox.stub(fakeTimer, 'setInterval')).callThrough();
const period = 50;
const state = { index: 0, period };
type State = typeof state;
Expand All @@ -92,7 +92,7 @@ describe('Scheduler.asap', () => {
const sandbox = sinon.createSandbox();
const fakeTimer = sandbox.useFakeTimers();
// callThrough is missing from the declarations installed by the typings tool in stable
const stubSetInterval = (<any> sandbox.stub(global, 'setInterval')).callThrough();
const stubSetInterval = (<any> sandbox.stub(fakeTimer, 'setInterval')).callThrough();
const period = 50;
const state = { index: 0, period };
type State = typeof state;
Expand Down Expand Up @@ -148,6 +148,12 @@ describe('Scheduler.asap', () => {
}, 0, 0);
});

it('should schedule asap actions from a delayed one', (done) => {
asap.schedule(() => {
asap.schedule(() => { done(); });
}, 1);
});

it('should cancel the setImmediate if all scheduled actions unsubscribe before it executes', (done) => {
let asapExec1 = false;
let asapExec2 = false;
Expand Down
2 changes: 1 addition & 1 deletion src/internal/scheduler/AnimationFrameAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class AnimationFrameAction<T> extends AsyncAction<T> {
// cancel the requested animation frame and set the scheduled flag to
// undefined so the next AnimationFrameAction will request its own.
const { actions } = scheduler;
if (id != null && actions[actions.length - 1]?.id !== id) {
if (id != null && id === scheduler._scheduled && actions[actions.length - 1]?.id !== id) {
animationFrameProvider.cancelAnimationFrame(id as number);
scheduler._scheduled = undefined;
}
Expand Down
9 changes: 7 additions & 2 deletions src/internal/scheduler/AnimationFrameScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,13 @@ export class AnimationFrameScheduler extends AsyncScheduler {
// 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;
let flushId;
if (action) {
flushId = action.id;
} else {
flushId = this._scheduled;
this._scheduled = undefined;
}

const { actions } = this;
let error: any;
Expand Down

0 comments on commit 8bbfa4e

Please sign in to comment.