Skip to content

Commit

Permalink
[core] Do not clear undefined timer IDs (#5002)
Browse files Browse the repository at this point in the history
* Do not clear undefined timeouts

* Add test

* Changeset

* tweak tests

---------

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
  • Loading branch information
davidkpiano and Andarist committed Jul 30, 2024
1 parent cc0a28d commit 9877d54
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-horses-pay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'xstate': patch
---

Fix an issue where `clearTimeout(undefined)` was sometimes being called, which can cause errors for some clock implementations. See https://github.com/statelyai/xstate/issues/5001 for details.
4 changes: 3 additions & 1 deletion packages/core/src/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ export function createSystem<T extends ActorSystemInfo>(
delete timerMap[scheduledEventId];
delete system._snapshot._scheduledEvents[scheduledEventId];

clock.clearTimeout(timeout);
if (timeout !== undefined) {
clock.clearTimeout(timeout);
}
},
cancelAll: (actorRef) => {
for (const scheduledEventId in system._snapshot._scheduledEvents) {
Expand Down
40 changes: 36 additions & 4 deletions packages/core/test/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3384,7 +3384,7 @@ describe('raise', () => {
});

describe('cancel', () => {
it('should be possible to cancel a raised delayed event', () => {
it('should be possible to cancel a raised delayed event', async () => {
const machine = createMachine({
initial: 'a',
states: {
Expand All @@ -3405,11 +3405,18 @@ describe('cancel', () => {

const actor = createActor(machine).start();

// This should raise the 'RAISED' event after 1ms
actor.send({ type: 'NEXT' });

// This should cancel the 'RAISED' event
actor.send({ type: 'CANCEL' });

setTimeout(() => {
expect(actor.getSnapshot().value).toBe('a');
}, 10);
await new Promise<void>((res) => {
setTimeout(() => {
expect(actor.getSnapshot().value).toBe('a');
res();
}, 10);
});
});

it('should cancel only the delayed event in the machine that scheduled it when canceling the event with the same ID in the machine that sent it first', async () => {
Expand Down Expand Up @@ -3507,6 +3514,31 @@ describe('cancel', () => {
expect(fooSpy).toHaveBeenCalledTimes(1);
expect(barSpy).not.toHaveBeenCalled();
});

it('should not try to clear an undefined timeout when canceling an unscheduled timer', async () => {
const spy = jest.fn();

const machine = createMachine({
on: {
FOO: {
actions: cancel('foo')
}
}
});

const actorRef = createActor(machine, {
clock: {
setTimeout,
clearTimeout: spy
}
}).start();

actorRef.send({
type: 'FOO'
});

expect(spy.mock.calls.length).toBe(0);
});
});

describe('assign action order', () => {
Expand Down
32 changes: 31 additions & 1 deletion packages/core/test/after.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createMachine, createActor } from '../src/index.ts';
import { sleep } from '@xstate-repo/jest-utils';
import { createMachine, createActor, cancel } from '../src/index.ts';

const lightMachine = createMachine({
id: 'light',
Expand Down Expand Up @@ -43,6 +44,35 @@ describe('delayed transitions', () => {
expect(actorRef.getSnapshot().value).toBe('yellow');
});

it('should not try to clear an undefined timeout when exiting source state of a delayed transition', async () => {
// https://github.com/statelyai/xstate/issues/5001
const spy = jest.fn();

const machine = createMachine({
initial: 'green',
states: {
green: {
after: {
1: 'yellow'
}
},
yellow: {}
}
});

const actorRef = createActor(machine, {
clock: {
setTimeout,
clearTimeout: spy
}
}).start();

// when the after transition gets executed it tries to clear its own timer when exiting its source state
await sleep(5);
expect(actorRef.getSnapshot().value).toBe('yellow');
expect(spy.mock.calls.length).toBe(0);
});

it('should format transitions properly', () => {
const greenNode = lightMachine.states.green;

Expand Down

0 comments on commit 9877d54

Please sign in to comment.