Fixed issues around reentrancy during external transitions (#3677)

* Fixed issues around reentrancy during external transitions

* Add more tests

* Convert a test to use the introduced helper

* add changeset

.changeset/large-snails-decide.md

@@ -0,0 +1,5 @@
+---
+'xstate': patch
+---
+
+Fixed an issue with targeted ancestors not being correctly exited when reented during external transitions.

.changeset/rich-monkeys-relate.md

@@ -0,0 +1,5 @@
+---
+'xstate': patch
+---
+
+Fixed an issue with the _active_ descendants of the targeted ancestor not being correctly reentered during external transitions.

packages/core/src/StateNode.ts

@@ -837,15 +837,13 @@ class StateNode<
     if (!willTransition) {
       return this.next(state, _event);
     }
-    const entryNodes = flatten(stateTransitions.map((t) => t.entrySet));
 
     const configuration = flatten(
       Object.keys(transitionMap).map((key) => transitionMap[key].configuration)
     );
 
     return {
       transitions: enabledTransitions,
-      entrySet: entryNodes,
       exitSet: flatten(stateTransitions.map((t) => t.exitSet)),
       configuration,
       source: state,
@@ -939,7 +937,6 @@ class StateNode<
     if (!nextStateNodes.length) {
       return {
         transitions: [selectedTransition],
-        entrySet: [],
         exitSet: [],
         configuration: state.value ? [this] : [],
         source: state,
@@ -955,43 +952,43 @@ class StateNode<
 
     const isInternal = !!selectedTransition.internal;
 
-    const reentryNodes: StateNode<any, any, any, any, any>[] = [];
-
-    if (!isInternal) {
-      nextStateNodes.forEach((targetNode) => {
-        reentryNodes.push(...this.getExternalReentryNodes(targetNode));
-      });
-    }
-
     return {
       transitions: [selectedTransition],
-      entrySet: reentryNodes,
-      exitSet: isInternal ? [] : [this],
+      exitSet: isInternal
+        ? []
+        : flatten(
+            nextStateNodes.map((targetNode) =>
+              this.getPotentiallyReenteringNodes(targetNode)
+            )
+          ),
       configuration: allNextStateNodes,
       source: state,
       actions
     };
   }
 
-  private getExternalReentryNodes(
+  // even though the name of this function mentions reentry nodes
+  // we are pushing its result into `exitSet`
+  // that's because what we exit might be reentered (it's an invariant of reentrancy)
+  private getPotentiallyReenteringNodes(
     targetNode: StateNode<TContext, any, TEvent, any, any, any>
   ): Array<StateNode<TContext, any, TEvent, any, any, any>> {
-    if (this.order > targetNode.order) {
-      return [targetNode];
+    if (this.order < targetNode.order) {
+      return [this];
     }
 
     const nodes: Array<StateNode<TContext, any, TEvent, any, any, any>> = [];
     let marker:
       | StateNode<TContext, any, TEvent, any, any, any>
-      | undefined = targetNode;
+      | undefined = this;
     let possibleAncestor: StateNode<
       TContext,
       any,
       TEvent,
       any,
       any,
       any
-    > = this;
+    > = targetNode;
 
     while (marker && marker !== possibleAncestor) {
       nodes.push(marker);
@@ -1015,17 +1012,20 @@ class StateNode<
     prevState?: State<TContext, any, any, any, any>,
     predictableExec?: PredictableActionArgumentsExec
   ): Array<Array<ActionObject<TContext, TEvent>>> {
-    const prevConfig = getConfiguration(
-      [],
-      prevState ? this.getStateNodes(prevState.value) : [this]
-    );
+    const prevConfig = prevState
+      ? getConfiguration([], this.getStateNodes(prevState.value))
+      : [];
+    const entrySet = new Set<StateNode<any, any, any, any, any, any>>();
 
-    for (const sn of resolvedConfig) {
+    for (const sn of Array.from(resolvedConfig).sort(
+      (a, b) => a.order - b.order
+    )) {
       if (
         !has(prevConfig, sn) ||
-        (has(transition.entrySet, sn.parent) && !has(transition.entrySet, sn))
+        has(transition.exitSet, sn) ||
+        (sn.parent && entrySet.has(sn.parent))
       ) {
-        transition.entrySet.push(sn);
+        entrySet.add(sn);
       }
     }
     for (const sn of prevConfig) {
@@ -1034,8 +1034,13 @@ class StateNode<
       }
     }
 
+    transition.exitSet.sort((a, b) => b.order - a.order);
+
+    const entryStates = Array.from(entrySet).sort((a, b) => a.order - b.order);
+    const exitStates = new Set(transition.exitSet);
+
     const doneEvents = flatten(
-      transition.entrySet.map((sn) => {
+      entryStates.map((sn) => {
         const events: DoneEventObject[] = [];
 
         if (sn.type !== 'final') {
@@ -1074,13 +1079,7 @@ class StateNode<
       })
     );
 
-    transition.exitSet.sort((a, b) => b.order - a.order);
-    transition.entrySet.sort((a, b) => a.order - b.order);
-
-    const entryStates = new Set(transition.entrySet);
-    const exitStates = new Set(transition.exitSet);
-
-    const entryActions = Array.from(entryStates)
+    const entryActions = entryStates
       .map((stateNode) => {
         const entryActions = stateNode.onEntry;
         const invokeActions = stateNode.activities.map((activity) =>
@@ -1192,7 +1191,6 @@ class StateNode<
     ) || {
       transitions: [],
       configuration: [],
-      entrySet: [],
       exitSet: [],
       source: currentState,
       actions: []
@@ -1675,7 +1673,6 @@ class StateNode<
     return this.resolveTransition(
       {
         configuration,
-        entrySet: configuration,
         exitSet: [],
         transitions: [],
         source: undefined,

packages/core/src/types.ts

@@ -1134,7 +1134,6 @@ export interface ActivityMap {
 export interface StateTransition<TContext, TEvent extends EventObject> {
   transitions: Array<TransitionDefinition<TContext, TEvent>>;
   configuration: Array<StateNode<TContext, any, TEvent, any, any, any>>;
-  entrySet: Array<StateNode<TContext, any, TEvent, any, any, any>>;
   exitSet: Array<StateNode<TContext, any, TEvent, any, any, any>>;
   /**
    * The source state that preceded the transition.

packages/core/test/actions.test.ts

@@ -463,6 +463,7 @@ describe('entry/exit actions', () => {
 
       expect(flushTracked()).toEqual([
         'exit: loaded.idle',
+        'exit: loaded',
         'enter: loaded',
         'enter: loaded.idle'
       ]);
@@ -647,6 +648,7 @@ describe('entry/exit actions', () => {
       expect(flushTracked()).toEqual([
         'exit: A.A2.A2_child',
         'exit: A.A2',
+        'exit: A',
         'enter: A',
         'enter: A.A1'
       ]);
@@ -725,9 +727,115 @@ describe('entry/exit actions', () => {
         'exit: a.a1',
         'exit: a',
         'enter: a',
+        'enter: a.a1',
+        'enter: a.a1.a11'
+      ]);
+    });
+
+    it('should reenter leaf state during its self-transition', () => {
+      const m = createMachine({
+        initial: 'a',
+        states: {
+          a: {
+            initial: 'a1',
+            states: {
+              a1: {
+                on: {
+                  EV: 'a1'
+                }
+              }
+            }
+          }
+        }
+      });
+
+      const flushTracked = trackEntries(m);
+
+      const service = interpret(m).start();
+
+      flushTracked();
+      service.send('EV');
+
+      expect(flushTracked()).toEqual(['exit: a.a1', 'enter: a.a1']);
+    });
+
+    it('should not enter exited state when targeting its ancestor and when its former descendant gets selected through initial state', () => {
+      const m = createMachine({
+        initial: 'a',
+        states: {
+          a: {
+            id: 'parent',
+            initial: 'a1',
+            states: {
+              a1: {
+                on: {
+                  EV: 'a2'
+                }
+              },
+              a2: {
+                on: {
+                  EV: '#parent'
+                }
+              }
+            }
+          }
+        }
+      });
+
+      const flushTracked = trackEntries(m);
+
+      const service = interpret(m).start();
+      service.send('EV');
+
+      flushTracked();
+      service.send('EV');
+
+      expect(flushTracked()).toEqual([
+        'exit: a.a2',
+        'exit: a',
+        'enter: a',
         'enter: a.a1'
       ]);
     });
+
+    it('should not enter exited state when targeting its ancestor and when its latter descendant gets selected through initial state', () => {
+      const m = createMachine({
+        initial: 'a',
+        states: {
+          a: {
+            id: 'parent',
+            initial: 'a2',
+            states: {
+              a1: {
+                on: {
+                  EV: '#parent'
+                }
+              },
+              a2: {
+                on: {
+                  EV: 'a1'
+                }
+              }
+            }
+          }
+        }
+      });
+
+      const flushTracked = trackEntries(m);
+
+      const service = interpret(m).start();
+      service.send('EV');
+
+      flushTracked();
+      service.send('EV');
+
+      expect(flushTracked()).toEqual([
+        'exit: a.a1',
+        'exit: a',
+        'enter: a',
+        'enter: a.a2'
+      ]);
+    });
   });
 
   describe('parallel states', () => {
@@ -765,8 +873,6 @@ describe('entry/exit actions', () => {
     });
 
     it('should reenter parallel region when a parallel state gets reentered while targeting another region', () => {
-      const actions: string[] = [];
-
       const machine = createMachine({
         initial: 'ready',
         states: {
@@ -776,13 +882,8 @@ describe('entry/exit actions', () => {
               FOO: '#cameraOff'
             },
             states: {
-              devicesInfo: {
-                entry: () => actions.push('entry devicesInfo'),
-                exit: () => actions.push('exit devicesInfo')
-              },
+              devicesInfo: {},
               camera: {
-                entry: () => actions.push('entry camera'),
-                exit: () => actions.push('exit camera'),
                 initial: 'on',
                 states: {
                   on: {},
@@ -796,16 +897,22 @@ describe('entry/exit actions', () => {
         }
       });
 
+      const flushTracked = trackEntries(machine);
+
       const service = interpret(machine).start();
 
-      actions.length = 0;
+      flushTracked();
       service.send('FOO');
 
-      expect(actions).toEqual([
-        'exit camera',
-        'exit devicesInfo',
-        'entry devicesInfo',
-        'entry camera'
+      expect(flushTracked()).toEqual([
+        'exit: ready.camera.on',
+        'exit: ready.camera',
+        'exit: ready.devicesInfo',
+        'exit: ready',
+        'enter: ready',
+        'enter: ready.devicesInfo',
+        'enter: ready.camera',
+        'enter: ready.camera.off'
       ]);
     });
   });