Avoid selecting always transitions when the preceeding transition doe…

…sn't change the state (#4371)

* Do not select the same always transitions after executimg them

* Move the filtering if statement to a more correct place

* only ignore action-less eventless transitions

* Add failing test

* Switch to comparing previous and next states

* test executation count as well

* remove irrelevant bit from a test case

* add changeset

* Add assign test

* tweak behavior

---------

Co-authored-by: David Khourshid <[email protected]>

.changeset/rich-ladybugs-love.md

@@ -0,0 +1,7 @@
+---
+'xstate': major
+---
+
+Changed behavior of `always` transitions. Previously they were always selected after selecting any transition (including the `always` transitions). Because of that it was relatively easy to create an infinite loop using them.
+
+Now they are no longer selected if the preceeding transition doesn't change the state of a machine.

packages/core/src/stateUtils.ts

@@ -1603,8 +1603,17 @@ export function macrostep(
     states.push(nextState);
   }
 
+  let shouldSelectEventlessTransitions = true;
+
   while (nextState.status === 'active') {
-    let enabledTransitions = selectEventlessTransitions(nextState, nextEvent);
+    let enabledTransitions: AnyTransitionDefinition[] =
+      shouldSelectEventlessTransitions
+        ? selectEventlessTransitions(nextState, nextEvent)
+        : [];
+
+    // eventless transitions should always be selected after selecting *regular* transitions
+    // by assigning `undefined` to `previousState` we ensure that `shouldSelectEventlessTransitions` gets always computed to true in such a case
+    const previousState = enabledTransitions.length ? nextState : undefined;
 
     if (!enabledTransitions.length) {
       if (!internalQueue.length) {
@@ -1613,6 +1622,7 @@ export function macrostep(
       nextEvent = internalQueue.shift()!;
       enabledTransitions = selectTransitions(nextEvent, nextState);
     }
+
     nextState = microstep(
       enabledTransitions,
       nextState,
@@ -1621,7 +1631,7 @@ export function macrostep(
       false,
       internalQueue
     );
-
+    shouldSelectEventlessTransitions = nextState !== previousState;
     states.push(nextState);
   }
 

packages/core/test/transient.test.ts

@@ -735,4 +735,164 @@ describe('transient states (eventless transitions)', () => {
     const service = createActor(machine).start();
     service.send({ type: 'EVENT', value: 42 });
   });
+
+  it("shouldn't end up in an infinite loop when selecting the fallback target", () => {
+    const machine = createMachine({
+      initial: 'idle',
+      states: {
+        idle: {
+          on: {
+            event: 'active'
+          }
+        },
+        active: {
+          initial: 'a',
+          states: {
+            a: {},
+            b: {}
+          },
+          always: [
+            {
+              guard: () => false,
+              target: '.a'
+            },
+            {
+              target: '.b'
+            }
+          ]
+        }
+      }
+    });
+    const actorRef = createActor(machine).start();
+    actorRef.send({
+      type: 'event'
+    });
+
+    expect(actorRef.getSnapshot().value).toEqual({ active: 'b' });
+  });
+
+  it("shouldn't end up in an infinite loop when selecting a guarded target", () => {
+    const machine = createMachine({
+      initial: 'idle',
+      states: {
+        idle: {
+          on: {
+            event: 'active'
+          }
+        },
+        active: {
+          initial: 'a',
+          states: {
+            a: {},
+            b: {}
+          },
+          always: [
+            {
+              guard: () => true,
+              target: '.a'
+            },
+            {
+              target: '.b'
+            }
+          ]
+        }
+      }
+    });
+    const actorRef = createActor(machine).start();
+    actorRef.send({
+      type: 'event'
+    });
+
+    expect(actorRef.getSnapshot().value).toEqual({ active: 'a' });
+  });
+
+  it("shouldn't end up in an infinite loop when executing a fire-and-forget action that doesn't change state", () => {
+    let count = 0;
+    const machine = createMachine({
+      initial: 'idle',
+      states: {
+        idle: {
+          on: {
+            event: 'active'
+          }
+        },
+        active: {
+          initial: 'a',
+          states: {
+            a: {}
+          },
+          always: [
+            {
+              actions: () => {
+                count++;
+                if (count > 5) {
+                  throw new Error('Infinite loop detected');
+                }
+              },
+              target: '.a'
+            }
+          ]
+        }
+      }
+    });
+
+    const actorRef = createActor(machine);
+
+    actorRef.start();
+    actorRef.send({
+      type: 'event'
+    });
+
+    expect(actorRef.getSnapshot().value).toEqual({ active: 'a' });
+    expect(count).toBe(1);
+  });
+
+  it('should loop (but not infinitely) for assign actions', () => {
+    const machine = createMachine({
+      context: { count: 0 },
+      initial: 'counting',
+      states: {
+        counting: {
+          always: {
+            guard: ({ context }) => context.count < 5,
+            actions: assign({ count: ({ context }) => context.count + 1 })
+          }
+        }
+      }
+    });
+
+    const actorRef = createActor(machine).start();
+
+    expect(actorRef.getSnapshot().context.count).toEqual(5);
+  });
+
+  it("should execute an always transition after a raised transition even if that raised transition doesn't change the state", () => {
+    const spy = jest.fn();
+    let counter = 0;
+    const machine = createMachine({
+      always: {
+        actions: () => spy(counter)
+      },
+      on: {
+        EV: {
+          actions: raise({ type: 'RAISED' })
+        },
+        RAISED: {
+          actions: () => {
+            ++counter;
+          }
+        }
+      }
+    });
+    const actorRef = createActor(machine).start();
+    spy.mockClear();
+    actorRef.send({ type: 'EV' });
+
+    expect(spy.mock.calls).toEqual([
+      // called in response to the `EV` event
+      [0],
+      // called in response to the `RAISED` event
+      [1]
+    ]);
+  });
 });