[core] Add warning and stop forwarding when forwarding to undefined #3178
Conversation
🦋 Changeset detectedLatest commit: 3f265c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3f265c0:
|
👇 Click on the image for a new way to code review
Legend |
| it('should not cause an infinite loop when forwarding to undefined', () => { | ||
| const machine = createMachine({ | ||
| on: { | ||
| '*': { cond: () => true, actions: forwardTo(undefined as any) } |
There was a problem hiding this comment.
This is what was causing the infinite loop...
packages/core/src/interpreter.ts
Outdated
| false, | ||
| `Attempted to forward event "${sendAction.event.type}" to undefined actor` | ||
| ); | ||
| break; | ||
| } else { | ||
| this.send( |
There was a problem hiding this comment.
And the infinite loop was being caused due to this line, which sends the event to self if sendAction.to is undefined.
packages/core/src/interpreter.ts
Outdated
| @@ -872,6 +872,13 @@ export class Interpreter< | |||
| } else { | |||
| if (sendAction.to) { | |||
| this.sendTo(sendAction._event, sendAction.to); | |||
| } else if (sendAction.sendType === 'forward') { | |||
There was a problem hiding this comment.
However, when we're forwarding an event, it should never be forwarded to self! So we need a way of identifying that the send action is a "forwarding" action, without breaking any behavior (hence sendAction.sendType === 'forward')
packages/core/src/actions.ts
Outdated
| @@ -563,7 +564,8 @@ export function forwardTo<TContext, TEvent extends EventObject>( | |||
| ): SendAction<TContext, TEvent, AnyEventObject> { | |||
| return send<TContext, TEvent>((_, event) => event, { | |||
| ...options, | |||
| to: target | |||
| to: target, | |||
| sendType: 'forward' | |||
There was a problem hiding this comment.
To do this, we add this little piece of sendType: 'forward' metadata to the action object created by forwardTo(...)
packages/core/src/actions.ts
Outdated
| @@ -205,7 +205,8 @@ export function send< | |||
| ? options.id | |||
| : isFunction(event) | |||
| ? event.name | |||
| : (getEventType<TSentEvent>(event) as string) | |||
| : (getEventType<TSentEvent>(event) as string), | |||
| sendType: options?.sendType | |||
There was a problem hiding this comment.
Make sure we're adding this on the action object...
packages/core/test/actions.test.ts
Outdated
| const service = interpret(machine).start(); | ||
|
|
||
| // This is testing for an infinite loop | ||
| service.send('TEST'); |
There was a problem hiding this comment.
And all should be well! Without adding that metadata, this would cause an infinite loop and this test would timeout (or worse, just hang).
This test completing is the success criterion.
|
Click "Tour" to see this code tour in CodeSee
|
Andarist
deleted the
3177-footgun-forwardto-causes-infinite-loop-when-you-pass-undefined
branch
This PR:
sendTypeproperty tosendactions to tell what "kind" of send action it is (plain send, forwarding)undefinedforforwardTo, which will prevent an infinite loop (which is already prevented in the types)undefined