Skip to content

Commit

Permalink
Fixed an issue with parallel transition sources not being reentered c…
Browse files Browse the repository at this point in the history
…orrectly (#4368)

* Fixed an issue with parallel transition sources not being reentered correctly

* Fixed root reentrancy

* add changeset
  • Loading branch information
Andarist committed Nov 2, 2023
1 parent 21c98f6 commit 5393e82
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 29 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-beans-mate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'xstate': patch
---

Fixed an issue with parallel regions sometimes not being correctly reentered when taking transitions targeting other parallel regions.
79 changes: 51 additions & 28 deletions packages/core/src/stateUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -864,21 +864,15 @@ export function removeConflictingTransitions(
return Array.from(filteredTransitions);
}

function findLCCA(stateNodes: Array<AnyStateNode>): AnyStateNode {
const [head] = stateNodes;

let current = getPathFromRootToNode(head);
let candidates: Array<AnyStateNode> = [];

for (const stateNode of stateNodes) {
const path = getPathFromRootToNode(stateNode);

candidates = current.filter((sn) => path.includes(sn));
current = candidates;
candidates = [];
function findLeastCommonAncestor(
stateNodes: Array<AnyStateNode>
): AnyStateNode | undefined {
const [head, ...tail] = stateNodes;
for (const ancestor of getProperAncestors(head, undefined)) {
if (tail.every((sn) => isDescendant(sn, ancestor))) {
return ancestor;
}
}

return current[current.length - 1];
}

function getEffectiveTargetStates(
Expand Down Expand Up @@ -933,9 +927,18 @@ function getTransitionDomain(
return transition.source;
}

const lcca = findLCCA(targetStates.concat(transition.source));
const lca = findLeastCommonAncestor(targetStates.concat(transition.source));

if (lca) {
return lca;
}

// at this point we know that it's a root transition since LCA couldn't be found
if (transition.reenter) {
return;
}

return lcca;
return transition.source.machine.root;
}

function computeExitSet(
Expand Down Expand Up @@ -1248,12 +1251,16 @@ function computeEntrySet(
}
const targetStates = getEffectiveTargetStates(t, historyValue);
for (const s of targetStates) {
const ancestors = getProperAncestors(s, domain);
if (domain?.type === 'parallel') {
ancestors.push(domain!);
}
addAncestorStatesToEnter(
s,
domain,
statesToEnter,
historyValue,
statesForDefaultEntry
statesForDefaultEntry,
ancestors,
!t.source.parent && t.reenter ? undefined : domain
);
}
}
Expand Down Expand Up @@ -1282,7 +1289,7 @@ function addDescendantStatesToEnter<
);
}
for (const s of historyStateNodes) {
addAncestorStatesToEnter(
addProperAncestorStatesToEnter(
s,
stateNode.parent!,
statesToEnter,
Expand Down Expand Up @@ -1311,7 +1318,7 @@ function addDescendantStatesToEnter<
}

for (const s of historyDefaultTransition.target) {
addAncestorStatesToEnter(
addProperAncestorStatesToEnter(
s,
stateNode,
statesToEnter,
Expand All @@ -1335,7 +1342,7 @@ function addDescendantStatesToEnter<
statesToEnter
);

addAncestorStatesToEnter(
addProperAncestorStatesToEnter(
initialState,
stateNode,
statesToEnter,
Expand Down Expand Up @@ -1366,15 +1373,16 @@ function addDescendantStatesToEnter<
}

function addAncestorStatesToEnter(
stateNode: AnyStateNode,
toStateNode: AnyStateNode | undefined,
statesToEnter: Set<AnyStateNode>,
historyValue: HistoryValue<any, any>,
statesForDefaultEntry: Set<AnyStateNode>
statesForDefaultEntry: Set<AnyStateNode>,
ancestors: AnyStateNode[],
reentrancyDomain?: AnyStateNode
) {
const properAncestors = getProperAncestors(stateNode, toStateNode);
for (const anc of properAncestors) {
statesToEnter.add(anc);
for (const anc of ancestors) {
if (!reentrancyDomain || isDescendant(anc, reentrancyDomain)) {
statesToEnter.add(anc);
}
if (anc.type === 'parallel') {
for (const child of getChildren(anc).filter((sn) => !isHistoryNode(sn))) {
if (![...statesToEnter].some((s) => isDescendant(s, child))) {
Expand All @@ -1391,6 +1399,21 @@ function addAncestorStatesToEnter(
}
}

function addProperAncestorStatesToEnter(
stateNode: AnyStateNode,
toStateNode: AnyStateNode | undefined,
statesToEnter: Set<AnyStateNode>,
historyValue: HistoryValue<any, any>,
statesForDefaultEntry: Set<AnyStateNode>
) {
addAncestorStatesToEnter(
statesToEnter,
historyValue,
statesForDefaultEntry,
getProperAncestors(stateNode, toStateNode)
);
}

function exitStates(
currentState: AnyState,
event: AnyEventObject,
Expand Down
35 changes: 35 additions & 0 deletions packages/core/test/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,41 @@ describe('initial actions', () => {
]
`);
});

it('should execute actions of the initial transition when taking a root reentering self-transition', () => {
const spy = jest.fn();
const machine = createMachine({
id: 'root',
initial: {
target: 'a',
actions: spy
},
states: {
a: {
on: {
NEXT: 'b'
}
},
b: {}
},
on: {
REENTER: {
target: '#root',
reenter: true
}
}
});

const actorRef = createActor(machine).start();

actorRef.send({ type: 'NEXT' });
spy.mockClear();

actorRef.send({ type: 'REENTER' });

expect(spy).toHaveBeenCalledTimes(1);
expect(actorRef.getSnapshot().value).toEqual('a');
});
});

describe('actions on invalid transition', () => {
Expand Down
105 changes: 104 additions & 1 deletion packages/core/test/parallel.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createMachine, createActor, StateValue } from '../src/index.ts';
import { assign } from '../src/actions/assign.ts';
import { raise } from '../src/actions/raise.ts';
import { testMultiTransition } from './utils.ts';
import { testMultiTransition, trackEntries } from './utils.ts';

const composerMachine = createMachine({
initial: 'ReadOnly',
Expand Down Expand Up @@ -1172,4 +1172,107 @@ describe('parallel states', () => {

expect(service.getSnapshot().value).toBe('finished');
});

it('source parallel region should be reentered when a transition within it targets another parallel region (parallel root)', async () => {
const machine = createMachine({
type: 'parallel',
states: {
Operation: {
initial: 'Waiting',
states: {
Waiting: {
on: {
TOGGLE_MODE: {
target: '#Demo'
}
}
},
Fetching: {}
}
},
Mode: {
initial: 'Normal',
states: {
Normal: {},
Demo: {
id: 'Demo'
}
}
}
}
});

const flushTracked = trackEntries(machine);

const actor = createActor(machine);
actor.start();
flushTracked();

actor.send({ type: 'TOGGLE_MODE' });

expect(flushTracked()).toEqual([
'exit: Mode.Normal',
'exit: Mode',
'exit: Operation.Waiting',
'exit: Operation',
'enter: Operation',
'enter: Operation.Waiting',
'enter: Mode',
'enter: Mode.Demo'
]);
});

it('source parallel region should be reentered when a transition within it targets another parallel region (nested parallel)', async () => {
const machine = createMachine({
initial: 'a',
states: {
a: {
type: 'parallel',
states: {
Operation: {
initial: 'Waiting',
states: {
Waiting: {
on: {
TOGGLE_MODE: {
target: '#Demo'
}
}
},
Fetching: {}
}
},
Mode: {
initial: 'Normal',
states: {
Normal: {},
Demo: {
id: 'Demo'
}
}
}
}
}
}
});

const flushTracked = trackEntries(machine);

const actor = createActor(machine);
actor.start();
flushTracked();

actor.send({ type: 'TOGGLE_MODE' });

expect(flushTracked()).toEqual([
'exit: a.Mode.Normal',
'exit: a.Mode',
'exit: a.Operation.Waiting',
'exit: a.Operation',
'enter: a.Operation',
'enter: a.Operation.Waiting',
'enter: a.Mode',
'enter: a.Mode.Demo'
]);
});
});

0 comments on commit 5393e82

Please sign in to comment.