Make all transitions internal by default (#3756)

* Make all transitions internal by default * fixed tests * fixed things * add changeset
statelyai · Mar 17, 2023 · 67d5761 · 67d5761
1 parent 66bc88a
commit 67d5761
Show file tree

Hide file tree

Showing 18 changed files with 52 additions and 47 deletions.
diff --git a/.changeset/tidy-pots-double.md b/.changeset/tidy-pots-double.md
@@ -0,0 +1,7 @@
+---
+'xstate': major
+---
+
+All transitions became internal by default. The style of the `target` pattern (`.child`, `sibling`, `#id`) has now no effect on the transition type.
+
+Internal transitions don't reenter their source state when the target lies within it. You can still create external transitions (ones that reenter the source state under the mentioned circumstances) by explicitly setting `external: true` on the given transition.
diff --git a/packages/core/src/StateNode.ts b/packages/core/src/StateNode.ts
@@ -220,6 +220,7 @@ export class StateNode<
             source: this,
             actions: this.initial.actions,
             eventType: null as any,
+            external: false,
             toJSON: () => ({
               target: this.initial!.target!.map((t) => `#${t.id}`),
               source: `#${this.id}`,
@@ -449,7 +450,7 @@ export class StateNode<
           return !(
             !transition.target &&
             !transition.actions.length &&
-            transition.internal
+            !transition.external
           );
         })
         .map((transition) => transition.eventType)

diff --git a/packages/core/src/scxml.ts b/packages/core/src/scxml.ts
@@ -404,7 +404,7 @@ function toConfig(
           target: getTargets(targets),
           ...(value.elements ? executableContent(value.elements) : undefined),
           ...guardObject,
-          internal
+          external: !internal
         };
 
         if (eventType === NULL_EVENT) {

diff --git a/packages/core/src/stateUtils.ts b/packages/core/src/stateUtils.ts
@@ -363,17 +363,7 @@ export function formatTransition<
   }
 ): AnyTransitionDefinition {
   const normalizedTarget = normalizeTarget(transitionConfig.target);
-  const internal =
-    stateNode === stateNode.machine.root
-      ? true // always internal for root
-      : 'internal' in transitionConfig
-      ? transitionConfig.internal
-      : normalizedTarget
-      ? normalizedTarget.some(
-          (_target) =>
-            isString(_target) && _target[0] === stateNode.machine.delimiter
-        )
-      : true;
+  const external = transitionConfig.external ?? false;
   const { guards } = stateNode.machine.options;
   const target = resolveTarget(stateNode, normalizedTarget);
 
@@ -394,7 +384,7 @@ export function formatTransition<
       : undefined,
     target,
     source: stateNode,
-    internal,
+    external,
     eventType: transitionConfig.event,
     toJSON: () => ({
       ...transition,
@@ -537,6 +527,7 @@ export function formatInitialTransition<
       source: stateNode,
       actions: [],
       eventType: null as any,
+      external: false,
       target: resolvedTarget!,
       toJSON: () => ({
         ...transition,
@@ -996,7 +987,7 @@ function getTransitionDomain(
   }
 
   if (
-    transition.internal &&
+    !transition.external &&
     transition.source.type === 'compound' &&
     targetStates.every((targetStateNode) =>
       isDescendant(targetStateNode, transition.source)
@@ -1075,6 +1066,7 @@ export function microstep<
           {
             target: [...currentState.configuration].filter(isAtomicStateNode),
             source: machine.root,
+            external: true,
             actions: [],
             eventType: null as any,
             toJSON: null as any // TODO: fix

diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
@@ -310,7 +310,7 @@ export interface TransitionConfig<
 > {
   guard?: GuardConfig<TContext, TExpressionEvent>;
   actions?: BaseActions<TContext, TExpressionEvent, TEvent, TAction>;
-  internal?: boolean;
+  external?: boolean;
   target?: TransitionTarget | undefined;
   meta?: Record<string, any>;
   description?: string;
@@ -1431,6 +1431,7 @@ export interface TransitionDefinition<
   target: Array<StateNode<TContext, TEvent>> | undefined;
   source: StateNode<TContext, TEvent>;
   actions: BaseActionObject[];
+  external: boolean;
   guard?: GuardDefinition<TContext, TEvent>;
   eventType: TEvent['type'] | '*';
   toJSON: () => {

diff --git a/packages/core/test/actions.test.ts b/packages/core/test/actions.test.ts
@@ -548,7 +548,7 @@ describe('entry/exit actions', () => {
       expect(called).toBe(true);
     });
 
-    it('root entry/exit actions should not be called on root external transitions', () => {
+    it('root entry/exit actions should be called on root external transitions', () => {
       let entrySpy = jest.fn();
       let exitSpy = jest.fn();
 
@@ -559,7 +559,7 @@ describe('entry/exit actions', () => {
         on: {
           EVENT: {
             target: '#two',
-            internal: false
+            external: true
           }
         },
         initial: 'one',
@@ -578,8 +578,8 @@ describe('entry/exit actions', () => {
 
       service.send({ type: 'EVENT' });
 
-      expect(entrySpy).not.toHaveBeenCalled();
-      expect(exitSpy).not.toHaveBeenCalled();
+      expect(entrySpy).toHaveBeenCalled();
+      expect(exitSpy).toHaveBeenCalled();
     });
 
     describe('should ignore same-parent state actions (sparse)', () => {
@@ -781,15 +781,15 @@ describe('entry/exit actions', () => {
       ]);
     });
 
-    it('should enter all descendents when target is a descendent of current', () => {
+    it('should enter all descendents when target is a descendent of the source when using an external transition', () => {
       const machine = createMachine({
         initial: 'A',
         states: {
           A: {
             initial: 'A1',
             on: {
               NEXT: {
-                internal: false,
+                external: true,
                 target: '.A2'
               }
             },

diff --git a/packages/core/test/activities.test.ts b/packages/core/test/activities.test.ts
@@ -371,7 +371,7 @@ describe('invocations (activities)', () => {
             on: {
               NEXT: {
                 target: 'a',
-                internal: false
+                external: true
               }
             }
           }

diff --git a/packages/core/test/history.test.ts b/packages/core/test/history.test.ts
@@ -130,7 +130,10 @@ describe('history states', () => {
       states: {
         a: {
           on: {
-            REENTER: '#b_hist'
+            REENTER: {
+              target: '#b_hist',
+              external: true
+            }
           },
           initial: 'a1',
           states: {

diff --git a/packages/core/test/internalTransitions.test.ts b/packages/core/test/internalTransitions.test.ts
@@ -19,14 +19,14 @@ const wordMachine = createMachine({
         RIGHT_CLICK: '.right',
         RIGHT_CLICK_EXTERNAL: {
           target: '.right',
-          internal: false
+          external: true
         },
         CENTER_CLICK: '.center',
         JUSTIFY_CLICK: '.justify',
         RESET: 'direction', // explicit self-transition
         RESET_TO_CENTER: {
           target: 'direction.center',
-          internal: false
+          external: true
         }
       }
     }

diff --git a/packages/core/test/invoke.test.ts b/packages/core/test/invoke.test.ts
@@ -727,7 +727,6 @@ describe('invoke', () => {
         entry: () => entryActionsCount++,
         on: {
           UPDATE: {
-            internal: true,
             actions: () => {
               actionsCount++;
             }
@@ -3220,8 +3219,8 @@ describe('invoke', () => {
     service.send({ type: 'FINISH' });
     expect(disposed).toBe(true);
   });
-  // https://github.com/statelyai/xstate/issues/3072
-  it('root invocations should not restart on root external transitions', () => {
+
+  it('root invocations should restart on root external transitions', () => {
     let count = 0;
 
     const machine = createMachine({
@@ -3235,7 +3234,7 @@ describe('invoke', () => {
       on: {
         EVENT: {
           target: '#two',
-          internal: false
+          external: true
         }
       },
       initial: 'one',
@@ -3251,7 +3250,7 @@ describe('invoke', () => {
 
     service.send({ type: 'EVENT' });
 
-    expect(count).toEqual(1);
+    expect(count).toEqual(2);
   });
 });
 

diff --git a/packages/core/test/json.test.ts b/packages/core/test/json.test.ts
@@ -145,8 +145,8 @@ describe('json', () => {
           "actions": [],
           "event": "done.invoke.active:invocation[0]",
           "eventType": "done.invoke.active:invocation[0]",
+          "external": false,
           "guard": undefined,
-          "internal": false,
           "source": "#active",
           "target": [
             "#(machine).foo",
@@ -157,8 +157,8 @@ describe('json', () => {
           "actions": [],
           "event": "error.platform.active:invocation[0]",
           "eventType": "error.platform.active:invocation[0]",
+          "external": false,
           "guard": undefined,
-          "internal": false,
           "source": "#active",
           "target": [
             "#(machine).bar",
@@ -169,8 +169,8 @@ describe('json', () => {
           "actions": [],
           "event": "EVENT",
           "eventType": "EVENT",
+          "external": false,
           "guard": undefined,
-          "internal": false,
           "source": "#active",
           "target": [
             "#(machine).foo",

diff --git a/packages/core/test/predictableExec.test.ts b/packages/core/test/predictableExec.test.ts
@@ -515,7 +515,7 @@ describe('predictableExec', () => {
           on: {
             REENTER: {
               target: 'active',
-              internal: false
+              external: true
             }
           }
         }

diff --git a/packages/core/test/state.test.ts b/packages/core/test/state.test.ts
@@ -29,7 +29,7 @@ const exampleMachine = createMachine<any, Events>({
       on: {
         EXTERNAL: {
           target: 'one',
-          internal: false
+          external: true
         },
         INERT: {},
         INTERNAL: {

diff --git a/packages/xstate-inspect/test/inspect.test.ts b/packages/xstate-inspect/test/inspect.test.ts
@@ -294,7 +294,7 @@ describe('@xstate/inspect', () => {
         },
         {
           "sessionId": "x:7",
-          "state": "{"value":{},"done":false,"context":{"value":{"unsafe":"[unsafe]"}},"historyValue":{},"actions":[{"type":"xstate.assign","params":{"context":{"value":{"unsafe":"[unsafe]"}},"actions":[]}}],"event":{"type":"EV","value":{"unsafe":"[unsafe]"}},"_event":{"name":"EV","data":{"type":"EV","value":{"unsafe":"[unsafe]"}},"$$type":"scxml","type":"external"},"_sessionid":"x:7","_initial":false,"changed":true,"transitions":[{"actions":[{"type":"xstate.assign","params":{"assignment":{}}}],"event":"EV","source":"#(machine)","internal":true,"eventType":"EV"}],"children":{},"tags":[]}",
+          "state": "{"value":{},"done":false,"context":{"value":{"unsafe":"[unsafe]"}},"historyValue":{},"actions":[{"type":"xstate.assign","params":{"context":{"value":{"unsafe":"[unsafe]"}},"actions":[]}}],"event":{"type":"EV","value":{"unsafe":"[unsafe]"}},"_event":{"name":"EV","data":{"type":"EV","value":{"unsafe":"[unsafe]"}},"$$type":"scxml","type":"external"},"_sessionid":"x:7","_initial":false,"changed":true,"transitions":[{"actions":[{"type":"xstate.assign","params":{"assignment":{}}}],"event":"EV","source":"#(machine)","external":false,"eventType":"EV"}],"children":{},"tags":[]}",
           "type": "service.state",
         },
       ]

diff --git a/packages/xstate-scxml/src/index.ts b/packages/xstate-scxml/src/index.ts
@@ -71,7 +71,7 @@ export function transitionToSCXML(
       target: (transition.target || [])
         .map((stateNode) => stateNode.id)
         .join(' '),
-      type: transition.internal ? 'internal' : undefined
+      type: !transition.external ? 'internal' : undefined
     }),
     elements: elements.length ? elements : undefined
   };

diff --git a/packages/xstate-scxml/test/fixtures.test.ts b/packages/xstate-scxml/test/fixtures.test.ts
@@ -18,8 +18,8 @@ describe('toSCXML', () => {
         encoding: 'utf-8'
       });
 
-      const machine = require(`./fixtures/${testGroupName}/${testName}`)
-        .default;
+      const machine =
+        require(`./fixtures/${testGroupName}/${testName}`).default;
 
       it(`${testGroupName}/${testName}`, () => {
         expect(xml2js(toSCXML(machine))).toEqual(

diff --git a/packages/xstate-scxml/test/fixtures/actionSend/send1.ts b/packages/xstate-scxml/test/fixtures/actionSend/send1.ts
@@ -8,14 +8,18 @@ export default createMachine({
       on: {
         t: {
           target: '#b',
-          actions: actions.raise({ type: 's' })
+          actions: actions.raise({ type: 's' }),
+          external: true
         }
       }
     },
     b: {
       id: 'b',
       on: {
-        s: '#c'
+        s: {
+          target: '#c',
+          external: true
+        }
       }
     },
     c: {

diff --git a/packages/xstate-scxml/test/scxml.test.ts b/packages/xstate-scxml/test/scxml.test.ts
@@ -85,8 +85,8 @@ describe('scxml', () => {
 
   testGroupKeys.forEach((testGroupName) => {
     testGroups[testGroupName].forEach((testName) => {
-      const originalMachine = require(`./fixtures/${testGroupName}/${testName}`)
-        .default;
+      const originalMachine =
+        require(`./fixtures/${testGroupName}/${testName}`).default;
       const scxmlDefinition = toSCXML(originalMachine);
 
       const scxmlTest = JSON.parse(
@@ -209,8 +209,7 @@ const lightMachine = createMachine({
       on: {
         TIMER: 'green',
         POWER_OUTAGE: {
-          target: 'red',
-          internal: true
+          target: 'red'
         }
       },
       ...pedestrianStates
@@ -238,7 +237,6 @@ xdescribe('transition to SCXML', () => {
           on: {
             SOME_EVENT: {
               target: 'next',
-              internal: true,
               guard: () => true,
               actions: ['foo', 'bar']
             }