Skip to content

Commit

Permalink
Fixed an issue with actors not being reinstantiated correctly in a mi…
Browse files Browse the repository at this point in the history
…crostep
  • Loading branch information
Andarist committed Jun 1, 2022
1 parent ca51452 commit 59c48cb
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 43 deletions.
5 changes: 5 additions & 0 deletions .changeset/twelve-dingos-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'xstate': patch
---

Fixed an issue with actors not being reinstantiated correctly when an actor with the same ID was first stopped and then invoked/spawned again in the same microstep.
5 changes: 4 additions & 1 deletion packages/core/src/StateMachine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
resolveMicroTransition,
resolveStateValue,
toState,
transitionNode
transitionNode,
setChildren
} from './stateUtils';
import type {
AreAllImplementationsAssumedToBeProvided,
Expand Down Expand Up @@ -305,6 +306,8 @@ export class StateMachine<
preInitial._initial = true;
preInitial.actions.unshift(...actions);

setChildren(preInitial.children, actions);

return preInitial;
}

Expand Down
5 changes: 1 addition & 4 deletions packages/core/src/actions/invoke.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ export function invoke<
type,
params: {
...params,
id: params.id,
src: params.src,
ref: new ObservableActorRef(behavior, id),
meta
ref: new ObservableActorRef(behavior, id)
}
} as InvokeActionObject;
}
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/actions/stop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ export function stop<
{
actor
},
({ params, type }, context, _event) => {
const actorRefOrString = isFunction(params.actor)
({ params, type }, context, _event, { state }) => {
const actorRef = isFunction(params.actor)
? params.actor(context, _event.data)
: params.actor;
: state.children[params.actor];

return {
type,
params: { actor: actorRefOrString }
params: { actor: actorRef }
} as StopActionObject;
}
);
Expand Down
36 changes: 8 additions & 28 deletions packages/core/src/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ export class Interpreter<
[actionTypes.cancel]: (_ctx, _e, { action }) => {
this.cancel((action as CancelActionObject).params.sendId);
},
[actionTypes.invoke]: (_ctx, _e, { action, state }) => {
[actionTypes.invoke]: (_ctx, _e, { action }) => {
const {
id,
autoForward,
Expand All @@ -651,26 +651,16 @@ export class Interpreter<
return;
}
ref._parent = this; // TODO: fix
// If the actor will be stopped right after it's started
// (such as in transient states) don't bother starting the actor.
if (
state.actions.find((otherAction) => {
return (
otherAction.type === actionTypes.stop &&
(otherAction as StopActionObject).params.actor === id
);
})
) {
// If the actor didn't end up being in the state
// (eg. going through transient states could stop it) don't bother starting the actor.
if (!this.state.children[id]) {
return;
}
try {
if (autoForward) {
this.forwardTo.add(id);
}

// TODO: determine how this can be immutably updated
this.state.children[id] = ref;

ref.start?.();
} catch (err) {
this.send(error(id, err));
Expand All @@ -679,11 +669,9 @@ export class Interpreter<
},
[actionTypes.stop]: (_ctx, _e, { action }) => {
const { actor } = (action as StopActionObject).params;
const actorRef =
typeof actor === 'string' ? this.state.children[actor] : actor;

if (actorRef) {
this.stopChild(actorRef.name);
if (actor) {
this.stopChild(actor);
}
},
[actionTypes.log]: (_ctx, _e, { action }) => {
Expand Down Expand Up @@ -746,16 +734,8 @@ export class Interpreter<
return undefined;
}

private stopChild(childId: string): void {
const child = this.state.children[childId];
if (!child) {
return;
}

this.forwardTo.delete(childId);
// TODO: determine how this can be immutably updated
delete this.state.children[childId];

private stopChild(child: ActorRef<any, any>): void {
this.forwardTo.delete(child.name);
if (isFunction(child.stop)) {
child.stop();
}
Expand Down
22 changes: 17 additions & 5 deletions packages/core/src/stateUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
BaseActionObject,
EventObject,
InvokeActionObject,
StopActionObject,
StateValue,
TransitionConfig,
TransitionDefinition,
Expand Down Expand Up @@ -1588,8 +1589,6 @@ export function resolveMicroTransition<
return inertState as any;
}

const children = { ...currentState.children };

const resolvedConfiguration = willTransition
? Array.from(resolved.configuration)
: !currentState._initial
Expand All @@ -1605,6 +1604,9 @@ export function resolveMicroTransition<

const { context, actions: nonRaisedActions } = resolved;

const children = { ...currentState.children };
setChildren(children, nonRaisedActions);

const nextState = machine.createState({
value: getStateValue(machine.root, resolved.configuration),
context,
Expand All @@ -1627,7 +1629,14 @@ export function resolveMicroTransition<
context !== currentState.context;
nextState._internalQueue = resolved.internalQueue;

nextState.actions.forEach((action) => {
return nextState;
}

export function setChildren<
TContext extends MachineContext,
TEvent extends EventObject
>(children: State<TContext, TEvent>['children'], actions: BaseActionObject[]) {
actions.forEach((action) => {
if (
action.type === actionTypes.invoke &&
(action as InvokeActionObject).params.ref
Expand All @@ -1636,10 +1645,13 @@ export function resolveMicroTransition<
if (ref) {
children[ref.name] = ref;
}
} else if (action.type === actionTypes.stop) {
const ref = (action as StopActionObject).params.actor;
if (ref) {
delete children[ref.name];
}
}
});

return nextState;
}

function resolveActionsAndContext<
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ export interface DynamicStopActionObject<
export interface StopActionObject {
type: ActionTypes.Stop;
params: {
actor: string | ActorRef<any>;
actor: ActorRef<any>;
};
}

Expand Down
29 changes: 29 additions & 0 deletions packages/core/test/invoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3117,6 +3117,35 @@ describe('invoke', () => {
done();
}, 100);
});

it('should get reinstantiated after reentering the invoking state in a microstep', () => {
let invokeCount = 0;

const machine = createMachine({
initial: 'a',
states: {
a: {
invoke: {
src: () =>
fromCallback(() => {
invokeCount++;
})
},
on: {
GO_AWAY_AND_REENTER: 'b'
}
},
b: {
always: 'a'
}
}
});
const service = interpret(machine).start();

service.send({ type: 'GO_AWAY_AND_REENTER' });

expect(invokeCount).toBe(2);
});
});

describe('actors option', () => {
Expand Down

0 comments on commit 59c48cb

Please sign in to comment.