Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v5] Remove autoForward #3889

Merged
merged 7 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/fresh-garlics-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
'xstate': major
---

Autoforwarding events is no longer supported and the `autoForward` property has been removed.

Instead of autoforwarding, events should be explicitly sent to actors:

```diff
invoke: {
id: 'child',
src: 'someSrc',
- autoForward: true
},
// ...
on: {
// ...
+ EVENT_TO_FORWARD: {
+ actions: sendTo('child', (_, event) => event)
+ }
}
```
6 changes: 1 addition & 5 deletions packages/core/src/actions/invoke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function invoke<

resolvedInvokeAction.execute = (actorCtx) => {
const interpreter = actorCtx.self as AnyInterpreter;
const { id, autoForward, ref } = resolvedInvokeAction.params;
const { id, ref } = resolvedInvokeAction.params;
if (!ref) {
if (!IS_PRODUCTION) {
warn(
Expand All @@ -100,10 +100,6 @@ export function invoke<
return;
}
try {
if (autoForward) {
interpreter._forwardTo.add(actorRef);
}

actorRef.start?.();
} catch (err) {
interpreter.send(error(id, err));
Expand Down
14 changes: 0 additions & 14 deletions packages/core/src/interpreter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type {
ActorContext,
AnyActorRef,
AnyStateMachine,
ActorBehavior,
EventFromBehavior,
Expand Down Expand Up @@ -115,9 +114,6 @@ export class Interpreter<
*/
public sessionId: string;

// TODO: remove
public _forwardTo: Set<AnyActorRef> = new Set();

private _doneEvent?: DoneEvent;

public src?: string;
Expand Down Expand Up @@ -303,8 +299,6 @@ export class Interpreter<
}

private _process(event: SCXML.Event<TEvent>) {
this.forward(event);

try {
const nextState = this.behavior.transition(
this._state,
Expand Down Expand Up @@ -418,14 +412,6 @@ export class Interpreter<
this.mailbox.enqueue(_event);
}

// TODO: remove
private forward(event: SCXML.Event<TEvent>): void {
// The _forwardTo set will be empty for non-machine actors anyway
for (const child of this._forwardTo) {
child.send(event);
}
}

// TODO: make private (and figure out a way to do this within the machine)
public delaySend(
sendAction: SendActionObject | RaiseActionObject<any, any, any>
Expand Down
4 changes: 0 additions & 4 deletions packages/core/src/machine.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,6 @@
},
"src": {
"type": "string"
},
"autoForward": {
"type": "boolean",
"default": false
}
},
"required": ["type", "id", "src"],
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/scxml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,7 @@ function toConfig(

return {
...(element.attributes!.id && { id: element.attributes!.id as string }),
src: scxmlToMachine(content, options),
autoForward: element.attributes!.autoforward === 'true'
src: scxmlToMachine(content, options)
};
});

Expand Down
21 changes: 0 additions & 21 deletions packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,6 @@ export interface InvokeDefinition<
* The source of the actor's behavior to be invoked
*/
src: string;
/**
* If `true`, events sent to the parent service will be forwarded to the invoked service.
*
* Default: `false`
*/
autoForward?: boolean;

input?: Mapper<TContext, TEvent, any> | any;
/**
Expand Down Expand Up @@ -530,12 +524,6 @@ export interface InvokeConfig<
* The source of the machine to be invoked, or the machine itself.
*/
src: string | ActorBehavior<any, any>; // TODO: fix types
/**
* If `true`, events sent to the parent service will be forwarded to the invoked service.
*
* Default: `false`
*/
autoForward?: boolean;

input?: Mapper<TContext, TEvent, any> | any;
/**
Expand Down Expand Up @@ -1142,7 +1130,6 @@ export interface InvokeAction {
type: ActionTypes.Invoke;
src: string | ActorRef<any>;
id: string;
autoForward?: boolean;
exec?: undefined;
meta: MetaObject | undefined;
}
Expand All @@ -1160,7 +1147,6 @@ export interface InvokeActionObject extends BaseActionObject {
params: {
src: string | ActorRef<any>;
id: string;
autoForward?: boolean;
exec?: undefined;
ref?: ActorRef<any>;
meta: MetaObject | undefined;
Expand Down Expand Up @@ -1620,13 +1606,6 @@ export interface InterpreterOptions<_TActorBehavior extends AnyActorBehavior> {
*/
devTools?: boolean | DevToolsAdapter; // TODO: add enhancer options

/**
* If `true`, events from the parent will be sent to this interpreter.
*
* Default: `false`
*/
autoForward?: boolean;

sync?: boolean;

/**
Expand Down
168 changes: 7 additions & 161 deletions packages/core/test/invoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,65 +98,7 @@ const fetcherMachine = createMachine({
});

describe('invoke', () => {
it('should start services (external machines)', (done) => {
const childMachine = createMachine({
id: 'child',
initial: 'init',
states: {
init: {
entry: [sendParent({ type: 'INC' }), sendParent({ type: 'INC' })]
}
}
});

const someParentMachine = createMachine<{ count: number }>(
{
id: 'parent',
context: { count: 0 },
initial: 'start',
states: {
start: {
invoke: {
src: 'child',
id: 'someService',
autoForward: true
},
always: {
target: 'stop',
guard: (ctx) => ctx.count === 2
},
on: {
INC: {
actions: assign({ count: (ctx) => ctx.count + 1 })
}
}
},
stop: {
type: 'final'
}
}
},
{
actors: {
child: childMachine
}
}
);

const actor = interpret(someParentMachine).onDone(() => {
// 1. The 'parent' machine will enter 'start' state
// 2. The 'child' service will be run with ID 'someService'
// 3. The 'child' machine will enter 'init' state
// 4. The 'entry' action will be executed, which sends 'INC' to 'parent' machine twice
// 5. The context will be updated to increment count to 2
expect(actor.getSnapshot().context.count).toEqual(2);
done();
});

actor.start();
});

it('should forward events to services if autoForward: true', () => {
it('child can immediately respond to the parent with multiple events', () => {
const childMachine = createMachine({
id: 'child',
initial: 'init',
Expand Down Expand Up @@ -184,16 +126,17 @@ describe('invoke', () => {
start: {
invoke: {
src: 'child',
id: 'someService',
autoForward: true
id: 'someService'
},
always: {
target: 'stop',
guard: (ctx) => ctx.count === -3
},
on: {
DEC: { actions: assign({ count: (ctx) => ctx.count - 1 }) },
FORWARD_DEC: undefined
FORWARD_DEC: {
actions: sendTo('child', { type: 'FORWARD_DEC' })
}
}
},
stop: {
Expand All @@ -216,7 +159,7 @@ describe('invoke', () => {
service
.onDone(() => {
// 1. The 'parent' machine will not do anything (inert transition)
// 2. The 'FORWARD_DEC' event will be forwarded to the 'child' machine (autoForward: true)
// 2. The 'FORWARD_DEC' event will be "forwarded" to the 'child' machine
// 3. On the 'child' machine, the 'FORWARD_DEC' event sends the 'DEC' action to the 'parent' thrice
// 4. The context of the 'parent' machine will be updated from 2 to -1

Expand All @@ -227,102 +170,6 @@ describe('invoke', () => {
service.send({ type: 'FORWARD_DEC' });
});

it('should forward events to services if autoForward: true before processing them', (done) => {
const actual: string[] = [];

const childMachine = createMachine<{ count: number }>({
id: 'child',
context: { count: 0 },
initial: 'counting',
states: {
counting: {
on: {
INCREMENT: [
{
target: 'done',
guard: (ctx) => {
actual.push('child got INCREMENT');
return ctx.count >= 2;
},
actions: assign((ctx) => ({ count: ++ctx.count }))
},
{
target: undefined,
actions: assign((ctx) => ({ count: ++ctx.count }))
}
]
}
},
done: {
type: 'final',
data: (ctx) => ({ countedTo: ctx.count })
}
},
on: {
START: {
actions: () => {
throw new Error('Should not receive START action here.');
}
}
}
});

const parentMachine = createMachine<{ countedTo: number }>({
id: 'parent',
context: { countedTo: 0 },
initial: 'idle',
states: {
idle: {
on: {
START: 'invokeChild'
}
},
invokeChild: {
invoke: {
src: childMachine,
autoForward: true,
onDone: {
target: 'done',
actions: assign((_ctx, event) => ({
countedTo: event.data.countedTo
}))
}
},
on: {
INCREMENT: {
actions: () => {
actual.push('parent got INCREMENT');
}
}
}
},
done: {
type: 'final'
}
}
});

const service = interpret(parentMachine)
.onDone(() => {
expect(service.getSnapshot().context).toEqual({ countedTo: 3 });
expect(actual).toEqual([
'child got INCREMENT',
'parent got INCREMENT',
'child got INCREMENT',
'parent got INCREMENT',
'child got INCREMENT',
'parent got INCREMENT'
]);
done();
})
.start();

service.send({ type: 'START' });
service.send({ type: 'INCREMENT' });
service.send({ type: 'INCREMENT' });
service.send({ type: 'INCREMENT' });
});

it('should start services (explicit machine, invoke = config)', (done) => {
const childMachine = createMachine<{ userId: string | undefined }>({
id: 'fetch',
Expand Down Expand Up @@ -507,8 +354,7 @@ describe('invoke', () => {
start: {
invoke: {
src: 'child',
id: 'someService',
autoForward: true
id: 'someService'
},
on: {
STOP: 'stop'
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ describe('json', () => {
number: 0,
string: 'hello'
},
invoke: [{ id: 'invokeId', src: 'invokeSrc', autoForward: true }],
invoke: [{ id: 'invokeId', src: 'invokeSrc' }],
states: {
testActions: {
invoke: [{ id: 'invokeId', src: 'invokeSrc', autoForward: true }],
invoke: [{ id: 'invokeId', src: 'invokeSrc' }],
entry: [
'stringActionType',
{
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/scxml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ const testGroups: Record<string, string[]> = {
// 'test225.txml', // unique invokeids generated at invoke time
// 'test226.txml', // <invoke src="...">
'test228.txml',
'test229.txml',
// 'test230.txml', // Manual test (TODO: check)
// 'test229.txml', // autoForward not supported in v5
// 'test230.txml', // autoForward not supported in v5
'test232.txml',
// 'test233.txml', // <finalize> not implemented yet
// 'test234.txml', // <finalize> not implemented yet
Expand Down
Loading