Skip to content

Commit

Permalink
Fixed an issue with external transitions targeting ancestor states ca…
Browse files Browse the repository at this point in the history
…using entry actions being called on states between the source and the target that were not actually reentered (#3623)

* Re-enter only the target when an ancestor

* Add changeset

* Add tests

* Create a machine per test case

* Add exit actions to descendent test

* Refactor the added tests to make machines in them more focused on the given test case

* Update .changeset/mighty-phones-press.md

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
  • Loading branch information
arromeo and Andarist committed Nov 14, 2022
1 parent 44719c2 commit 163c255
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/mighty-phones-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'xstate': patch
---

Fixed an issue with external transitions targeting ancestor states. In such a case, `entry` actions were incorrectly called on the states between the source state and the target state for states that were not reentered within this transition.
19 changes: 15 additions & 4 deletions packages/core/src/StateNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -976,11 +976,22 @@ class StateNode<
private getExternalReentryNodes(
targetNode: StateNode<TContext, any, TEvent, any, any, any>
): Array<StateNode<TContext, any, TEvent, any, any, any>> {
if (this.order > targetNode.order) {
return [targetNode];
}

const nodes: Array<StateNode<TContext, any, TEvent, any, any, any>> = [];
let [marker, possibleAncestor]: [
StateNode<TContext, any, TEvent, any, any, any> | undefined,
StateNode<TContext, any, TEvent, any, any, any>
] = targetNode.order > this.order ? [targetNode, this] : [this, targetNode];
let marker:
| StateNode<TContext, any, TEvent, any, any, any>
| undefined = targetNode;
let possibleAncestor: StateNode<
TContext,
any,
TEvent,
any,
any,
any
> = this;

while (marker && marker !== possibleAncestor) {
nodes.push(marker);
Expand Down
134 changes: 134 additions & 0 deletions packages/core/test/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,140 @@ describe('entry/exit actions', () => {
).toEqual(['exit_walk', 'exit_red', 'enter_red', 'enter_walk']);
});

it('should exit current node and enter target node when target is not a descendent or ancestor of current', () => {
const actual: string[] = [];
const machine = createMachine({
initial: 'A',
states: {
A: {
exit: () => actual.push('exit_A'),
initial: 'A1',
states: {
A1: {
exit: () => actual.push('exit_A1'),
on: {
NEXT: '#sibling_descendant'
}
},
A2: {
entry: () => actual.push('enter_A2'),
initial: 'A2_child',
states: {
A2_child: {
id: 'sibling_descendant',
entry: () => actual.push('enter_A2_child')
}
}
}
}
}
}
});

const service = interpret(machine).start({ A: 'A1' });
service.send({ type: 'NEXT' });

expect(actual).toEqual(['exit_A1', 'enter_A2', 'enter_A2_child']);
});

it('should exit current node and enter target node when target is ancestor of current', () => {
const actual: string[] = [];
const machine = createMachine({
initial: 'A',
states: {
A: {
id: 'ancestor',
entry: () => actual.push('enter_A'),
exit: () => actual.push('exit_A'),
initial: 'A1',
states: {
A1: {
entry: () => actual.push('enter_A1'),
on: {
NEXT: 'A2'
}
},
A2: {
entry: () => actual.push('enter_A2'),
exit: () => actual.push('exit_A2'),
initial: 'A2_child',
states: {
A2_child: {
entry: () => actual.push('enter_A2_child'),
exit: () => actual.push('exit_A2_child'),
on: {
NEXT: '#ancestor'
}
}
}
}
}
}
}
});

const service = interpret(machine).start();
service.send({ type: 'NEXT' });

actual.length = 0;
service.send({ type: 'NEXT' });

expect(actual).toEqual([
'exit_A2_child',
'exit_A2',
'enter_A',
'enter_A1'
]);
});

it('should enter all descendents when target is a descendent of current', () => {
const actual: string[] = [];
const machine = createMachine({
initial: 'A',
states: {
A: {
initial: 'A1',
entry: () => actual.push('enter_A'),
exit: () => actual.push('exit_A'),
on: {
NEXT: {
internal: false,
target: '.A2'
}
},
states: {
A1: {
entry: () => actual.push('enter_A1'),
exit: () => actual.push('exit_A1')
},
A2: {
initial: 'A2a',
entry: () => actual.push('enter_A2'),
states: {
A2a: {
entry: () => actual.push('enter_A2a'),
exit: () => actual.push('exit_A2a')
}
}
}
}
}
}
});

const service = interpret(machine).start();
actual.length = 0;
service.send({ type: 'NEXT' });

expect(actual).toEqual([
'exit_A1',
'exit_A',
'enter_A',
'enter_A2',
'enter_A2a'
]);
});

it('should exit deep descendant during a self-transition', () => {
const actual: string[] = [];
const m = createMachine({
Expand Down

0 comments on commit 163c255

Please sign in to comment.