[core] Add warning for imperative built-in action calls (#4876)

* Add warning for imperative built-in action calls * Changeset * Remove spawnChild from warning list (used internally) * Handle errors * Update packages/core/src/stateUtils.ts Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Refactor * Move assignment --------- Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
statelyai · May 4, 2024 · 3f6a73b · 3f6a73b
1 parent 148d8fc
commit 3f6a73b
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 1 deletion.
diff --git a/.changeset/twenty-parrots-add.md b/.changeset/twenty-parrots-add.md
@@ -0,0 +1,17 @@
+---
+'xstate': patch
+---
+
+XState will now warn when calling built-in actions like `assign`, `sendTo`, `raise`, `emit`, etc. directly inside of a custom action. See https://stately.ai/docs/actions#built-in-actions for more details.
+
+```ts
+const machine = createMachine({
+  entry: () => {
+    // Will warn:
+    // "Custom actions should not call \`assign()\` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details."
+    assign({
+      // ...
+    });
+  }
+});
+```
diff --git a/jest.config.js b/jest.config.js
@@ -4,6 +4,7 @@ const { constants } = require('jest-config');
  * @type {import('@jest/types').Config.InitialOptions}
  */
 module.exports = {
+  prettierPath: null,
   setupFilesAfterEnv: ['@xstate-repo/jest-utils/setup'],
   transform: {
     [constants.DEFAULT_JS_PATTERN]: 'babel-jest',

diff --git a/packages/core/src/actions/assign.ts b/packages/core/src/actions/assign.ts
@@ -1,6 +1,7 @@
 import isDevelopment from '#is-development';
 import { cloneMachineSnapshot } from '../State.ts';
 import { Spawner, createSpawner } from '../spawn.ts';
+import { executingCustomAction } from '../stateUtils.ts';
 import type {
   ActionArgs,
   AnyActorScope,
@@ -156,6 +157,12 @@ export function assign<
   never,
   never
 > {
+  if (isDevelopment && executingCustomAction) {
+    console.warn(
+      'Custom actions should not call `assign()` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.'
+    );
+  }
+
   function assign(
     args: ActionArgs<TContext, TExpressionEvent, TEvent>,
     params: TParams

diff --git a/packages/core/src/actions/emit.ts b/packages/core/src/actions/emit.ts
@@ -1,4 +1,5 @@
 import isDevelopment from '#is-development';
+import { executingCustomAction } from '../stateUtils.ts';
 import {
   ActionArgs,
   AnyActorScope,
@@ -121,6 +122,12 @@ export function emit<
   never,
   TEmitted
 > {
+  if (isDevelopment && executingCustomAction) {
+    console.warn(
+      'Custom actions should not call `emit()` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.'
+    );
+  }
+
   function emit(
     args: ActionArgs<TContext, TExpressionEvent, TEvent>,
     params: TParams

diff --git a/packages/core/src/actions/raise.ts b/packages/core/src/actions/raise.ts
@@ -1,4 +1,5 @@
 import isDevelopment from '#is-development';
+import { executingCustomAction } from '../stateUtils.ts';
 import {
   ActionArgs,
   ActionFunction,
@@ -141,6 +142,12 @@ export function raise<
   TDelay,
   never
 > {
+  if (isDevelopment && executingCustomAction) {
+    console.warn(
+      'Custom actions should not call `raise()` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.'
+    );
+  }
+
   function raise(
     args: ActionArgs<TContext, TExpressionEvent, TEvent>,
     params: TParams

diff --git a/packages/core/src/actions/send.ts b/packages/core/src/actions/send.ts
@@ -1,6 +1,7 @@
 import isDevelopment from '#is-development';
 import { XSTATE_ERROR } from '../constants.ts';
 import { createErrorActorEvent } from '../eventUtils.ts';
+import { executingCustomAction } from '../stateUtils.ts';
 import {
   ActionArgs,
   ActionFunction,
@@ -232,6 +233,12 @@ export function sendTo<
   TDelay,
   never
 > {
+  if (isDevelopment && executingCustomAction) {
+    console.warn(
+      'Custom actions should not call `raise()` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.'
+    );
+  }
+
   function sendTo(
     args: ActionArgs<TContext, TExpressionEvent, TEvent>,
     params: TParams

diff --git a/packages/core/src/actions/spawnChild.ts b/packages/core/src/actions/spawnChild.ts
@@ -1,6 +1,7 @@
 import isDevelopment from '#is-development';
 import { cloneMachineSnapshot } from '../State.ts';
 import { ProcessingStatus, createActor } from '../createActor.ts';
+import { executingCustomAction } from '../stateUtils.ts';
 import {
   ActionArgs,
   ActionFunction,

diff --git a/packages/core/src/stateUtils.ts b/packages/core/src/stateUtils.ts
@@ -1491,6 +1491,10 @@ interface BuiltinAction {
   execute: (actorScope: AnyActorScope, params: unknown) => void;
 }
 
+export let executingCustomAction:
+  | ActionFunction<any, any, any, any, any, any, any, any, any>
+  | false = false;
+
 function resolveAndExecuteActionsWithContext(
   currentSnapshot: AnyMachineSnapshot,
   event: AnyEventObject,
@@ -1563,7 +1567,12 @@ function resolveAndExecuteActionsWithContext(
           params: actionParams
         }
       });
-      resolvedAction(actionArgs, actionParams);
+      try {
+        executingCustomAction = resolvedAction;
+        resolvedAction(actionArgs, actionParams);
+      } finally {
+        executingCustomAction = false;
+      }
     }
 
     if (!('resolve' in resolvedAction)) {

diff --git a/packages/core/test/actions.test.ts b/packages/core/test/actions.test.ts
@@ -1,11 +1,13 @@
 import { sleep } from '@xstate-repo/jest-utils';
 import {
   cancel,
+  emit,
   enqueueActions,
   log,
   raise,
   sendParent,
   sendTo,
+  spawnChild,
   stopChild
 } from '../src/actions.ts';
 import { CallbackActorRef, fromCallback } from '../src/actors/callback.ts';
@@ -3935,4 +3937,34 @@ describe('actions', () => {
       foo: 'bar'
     });
   });
+
+  it('should warn if called in custom action', () => {
+    const machine = createMachine({
+      entry: () => {
+        assign({});
+        raise({ type: '' });
+        sendTo('', { type: '' });
+        emit({ type: '' });
+      }
+    });
+
+    createActor(machine).start();
+
+    expect(console.warn).toMatchMockCallsInlineSnapshot(`
+[
+  [
+    "Custom actions should not call \`assign()\` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.",
+  ],
+  [
+    "Custom actions should not call \`raise()\` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.",
+  ],
+  [
+    "Custom actions should not call \`raise()\` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.",
+  ],
+  [
+    "Custom actions should not call \`emit()\` directly, as it is not imperative. See https://stately.ai/docs/actions#built-in-actions for more details.",
+  ],
+]
+`);
+  });
 });