Fixed sibling targets on root accidentally resolving the root's child…

…ren (#2881) * Improve target resolution algorithm * Add changesets * Fixed targets * Fix test * Remove changeset --------- Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
statelyai · Apr 8, 2023 · 2f45343 · 2f45343
1 parent 1dbf190
commit 2f45343
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 37 deletions.
diff --git a/.changeset/heavy-apes-tie.md b/.changeset/heavy-apes-tie.md
@@ -0,0 +1,20 @@
+---
+'xstate': major
+---
+
+Target resolution improvements: targeting sibling nodes from the root is no longer valid, since the root node has no siblings:
+
+```diff
+createMachine({
+  id: 'direction',
+  initial: 'left',
+  states: {
+    left: {},
+    right: {}
+  },
+  on: {
+-   LEFT_CLICK: 'left',
++   LEFT_CLICK: '.left'
+  }
+});
+```
diff --git a/packages/core/src/StateMachine.ts b/packages/core/src/StateMachine.ts
@@ -6,6 +6,7 @@ import { StateNode } from './StateNode.ts';
 import { interpret } from './interpreter.ts';
 import {
   getConfiguration,
+  getStateNodeByPath,
   getInitialConfiguration,
   getStateNodes,
   isInFinalState,
@@ -371,17 +372,19 @@ export class StateMachine<
   }
 
   public getStateNodeById(stateId: string): StateNode<TContext, TEvent> {
-    const resolvedStateId = isStateId(stateId)
-      ? stateId.slice(STATE_IDENTIFIER.length)
-      : stateId;
+    const fullPath = stateId.split(this.delimiter);
+    const relativePath = fullPath.slice(1);
+    const resolvedStateId = isStateId(fullPath[0])
+      ? fullPath[0].slice(STATE_IDENTIFIER.length)
+      : fullPath[0];
 
     const stateNode = this.idMap.get(resolvedStateId);
     if (!stateNode) {
       throw new Error(
         `Child state node '#${resolvedStateId}' does not exist on machine '${this.id}'`
       );
     }
-    return stateNode;
+    return getStateNodeByPath(stateNode, relativePath);
   }
 
   public get definition(): StateMachineDefinition<TContext, TEvent> {

diff --git a/packages/core/src/stateUtils.ts b/packages/core/src/stateUtils.ts
@@ -566,6 +566,10 @@ export function resolveTarget(
     if (!isString(target)) {
       return target;
     }
+    if (isStateId(target)) {
+      return stateNode.machine.getStateNodeById(target);
+    }
+
     const isInternalTarget = target[0] === stateNode.machine.delimiter;
     // If internal target is defined on machine,
     // do not include machine key on target
@@ -586,7 +590,9 @@ export function resolveTarget(
         );
       }
     } else {
-      return getStateNodeByPath(stateNode, resolvedTarget);
+      throw new Error(
+        `Invalid target: "${target}" is not a valid target from the root node. Did you mean ".${target}"?`
+      );
     }
   });
 }
@@ -668,7 +674,7 @@ export function getStateNode(
  *
  * @param statePath The string or string array relative path to the state node.
  */
-function getStateNodeByPath(
+export function getStateNodeByPath(
   stateNode: AnyStateNode,
   statePath: string | string[]
 ): AnyStateNode {

diff --git a/packages/core/test/actions.test.ts b/packages/core/test/actions.test.ts
@@ -2100,30 +2100,26 @@ describe('actions config', () => {
 
   it('should reference actions defined in actions parameter of machine options (entry actions)', () => {
     const spy = jest.fn();
-    const machine = createMachine(
-      {
-        initial: 'a',
-        states: {
-          a: {
-            on: {
-              EVENT: 'b'
-            }
-          },
-          b: {
-            entry: [
-              'definedAction',
-              { type: 'definedAction' },
-              'undefinedAction'
-            ]
+    const machine = createMachine({
+      initial: 'a',
+      states: {
+        a: {
+          on: {
+            EVENT: 'b'
           }
+        },
+        b: {
+          entry: ['definedAction', { type: 'definedAction' }, 'undefinedAction']
         }
       },
-      {
-        actions: {
-          definedAction: spy
-        }
+      on: {
+        E: '.a'
       }
-    );
+    }).provide({
+      actions: {
+        definedAction: spy
+      }
+    });
 
     const actor = interpret(machine).start();
     actor.send({ type: 'EVENT' });

diff --git a/packages/core/test/fixtures/id.ts b/packages/core/test/fixtures/id.ts
@@ -38,9 +38,7 @@ export const machine = createMachine({
             NEXT: '#A_foo'
           }
         },
-        dot_custom: {
-          id: 'B.dot'
-        }
+        dot: {}
       }
     }
   }

diff --git a/packages/core/test/id.test.ts b/packages/core/test/id.test.ts
@@ -42,9 +42,7 @@ const idMachine = createMachine({
             NEXT: '#A_foo'
           }
         },
-        dot: {
-          id: 'B.dot'
-        }
+        dot: {}
       }
     }
   }

diff --git a/packages/core/test/invalid.test.ts b/packages/core/test/invalid.test.ts
@@ -54,3 +54,22 @@ describe('invalid or resolved states', () => {
     );
   });
 });
+
+describe('invalid transition', () => {
+  it('should throw when attempting to create a machine with a sibling target on the root node', () => {
+    expect(() => {
+      createMachine({
+        id: 'direction',
+        initial: 'left',
+        states: {
+          left: {},
+          right: {}
+        },
+        on: {
+          LEFT_CLICK: 'left',
+          RIGHT_CLICK: 'right'
+        }
+      });
+    }).toThrowError(/invalid target/i);
+  });
+});
diff --git a/packages/core/test/invoke.test.ts b/packages/core/test/invoke.test.ts
@@ -327,7 +327,7 @@ describe('invoke', () => {
       },
       on: {
         SUCCESS: {
-          target: 'success',
+          target: '.success',
           guard: ({ event }) => {
             return event.data === 42;
           }

diff --git a/packages/core/test/parallel.test.ts b/packages/core/test/parallel.test.ts
@@ -759,7 +759,7 @@ describe('parallel states', () => {
         }
       },
       on: {
-        'to-A': 'A'
+        'to-A': '.A'
       }
     });
 

diff --git a/packages/core/test/types.test.ts b/packages/core/test/types.test.ts
@@ -502,7 +502,7 @@ describe('service-targets', () => {
     const machine = createMachine({
       invoke: {
         src: fromPromise(() => new Promise((resolve) => resolve(1))),
-        onDone: ['a', 'b']
+        onDone: ['.a', '.b']
       },
       initial: 'a',
       states: {
@@ -518,7 +518,7 @@ describe('service-targets', () => {
     const machine = createMachine({
       invoke: {
         src: fromPromise(() => new Promise((resolve) => resolve(1))),
-        onDone: [{ target: 'a' }, { target: 'b' }]
+        onDone: [{ target: '.a' }, { target: '.b' }]
       },
       initial: 'a',
       states: {
@@ -534,7 +534,7 @@ describe('service-targets', () => {
     const machine = createMachine({
       invoke: {
         src: fromPromise(() => new Promise((resolve) => resolve(1))),
-        onDone: [{ target: 'a' }, 'b']
+        onDone: [{ target: '.a' }, '.b']
       },
       initial: 'a',
       states: {