fix: animationFrameScheduler prematurely executing actions without delay

spec/schedulers/AnimationFrameScheduler-spec.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 import * as sinon from 'sinon';
-import { animationFrameScheduler, Subscription, merge } from 'rxjs';
+import {animationFrameScheduler, Subscription, merge, SchedulerAction} from 'rxjs';
 import { delay } from 'rxjs/operators';
 import { TestScheduler } from 'rxjs/testing';
 import { observableMatcher } from '../helpers/observableMatcher';
@@ -238,4 +238,49 @@ describe('Scheduler.animationFrame', () => {
       done();
     });
   });
+
+  it('should handle actions scheduled during flush before current action is rescheduled', (done) => {
+    const sandbox = sinon.createSandbox();
+
+    const result: string[] = [];
+    let reschedule = true;
+    function work(this: SchedulerAction<unknown>) {
+      result.push('work');
+      if (reschedule) {
+        animationFrameScheduler.schedule(() => result.push('task 1'));
+        animationFrameScheduler.schedule(() => result.push('task 2'));
+        this.schedule();
+        expect(result).to.deep.equal(['work']);
+        reschedule = false;
+      } else {
+        expect(result).to.deep.equal(['work', 'task 1', 'task 2', 'work']);
+        sandbox.restore();
+        done();
+      }
+    }
+    animationFrameScheduler.schedule(work);
+    expect(result).to.deep.equal([]);
+  });
+
+  it('should execute actions with delay separate from all other actions', () => {
+    const sandbox = sinon.createSandbox();
+    const timers = sandbox.useFakeTimers();
+    let rafCallback!: () => void;
+    const stub = sinon.stub(animationFrameProvider, 'requestAnimationFrame').callsFake((cb) => rafCallback = cb);
+
+    let asyncExecuted = false;
+    let animationFrameExecuted = false
+    animationFrameScheduler.schedule(() => asyncExecuted = true, 1);
+    animationFrameScheduler.schedule(() => animationFrameExecuted = true);
+
+    timers.tick(1);
+    expect(asyncExecuted).to.equal(true);
+    expect(animationFrameExecuted).to.equal(false);
+
+    rafCallback();
+    expect(animationFrameExecuted).to.equal(true);
+
+    stub.restore();
+    sandbox.restore();
+  });
 });

spec/schedulers/AsapScheduler-spec.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 import * as sinon from 'sinon';
-import { asapScheduler, Subscription, SchedulerAction, merge } from 'rxjs';
+import {asapScheduler, Subscription, SchedulerAction, merge, animationFrameScheduler} from 'rxjs';
 import { delay } from 'rxjs/operators';
 import { TestScheduler } from 'rxjs/testing';
 import { observableMatcher } from '../helpers/observableMatcher';
@@ -288,4 +288,27 @@ describe('Scheduler.asap', () => {
       done();
     });
   });
+
+  it('should handle actions scheduled during flush before current action is rescheduled', (done) => {
+    const sandbox = sinon.createSandbox();
+
+    const result: string[] = [];
+    let reschedule = true;
+    function work(this: SchedulerAction<unknown>) {
+      result.push('work');
+      if (reschedule) {
+        asapScheduler.schedule(() => result.push('task 1'));
+        asapScheduler.schedule(() => result.push('task 2'));
+        this.schedule();
+        expect(result).to.deep.equal(['work']);
+        reschedule = false;
+      } else {
+        expect(result).to.deep.equal(['work', 'task 1', 'task 2', 'work']);
+        sandbox.restore();
+        done();
+      }
+    }
+    asapScheduler.schedule(work);
+    expect(result).to.deep.equal([]);
+  });
 });

src/internal/scheduler/AnimationFrameAction.ts

@@ -27,10 +27,10 @@ export class AnimationFrameAction<T> extends AsyncAction<T> {
     if ((delay != null && delay > 0) || (delay == null && this.delay > 0)) {
       return super.recycleAsyncId(scheduler, id, delay);
     }
-    // If the scheduler queue has no remaining actions with the same async id,
+    // If the scheduler queue has no remaining actions for the next schedule,
     // 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)) {
+    if (scheduler._scheduled === id && !scheduler.actions.some((action) => action.id === id)) {
       animationFrameProvider.cancelAnimationFrame(id);
       scheduler._scheduled = undefined;
     }

src/internal/scheduler/AnimationFrameScheduler.ts

@@ -3,6 +3,20 @@ import { AsyncScheduler } from './AsyncScheduler';
 
 export class AnimationFrameScheduler extends AsyncScheduler {
   public flush(action?: AsyncAction<any>): void {
+    let error: any;
+
+    if (action) {
+      // This code path handles AsyncActions scheduled with delay.
+      // These are not executed from _scheduled nor part of the actions queue.
+      this._active = true;
+      error = action.execute(action.state, action.delay);
+      this._active = false;
+      if (error) {
+        throw error;
+      }
+      return;
+    }
+
     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
@@ -17,8 +31,7 @@ export class AnimationFrameScheduler extends AsyncScheduler {
     this._scheduled = undefined;
 
     const { actions } = this;
-    let error: any;
-    action = action || actions.shift()!;
+    action = actions.shift()!;
 
     do {
       if ((error = action.execute(action.state, action.delay))) {

src/internal/scheduler/AsapAction.ts

@@ -27,10 +27,10 @@ export class AsapAction<T> extends AsyncAction<T> {
     if ((delay != null && delay > 0) || (delay == null && this.delay > 0)) {
       return super.recycleAsyncId(scheduler, id, delay);
     }
-    // If the scheduler queue has no remaining actions with the same async id,
+    // If the scheduler queue has no remaining actions for the next schedule,
     // 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)) {
+    if (scheduler._scheduled === id && !scheduler.actions.some((action) => action.id === id)) {
       immediateProvider.clearImmediate(id);
       scheduler._scheduled = undefined;
     }