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] Make all transitions internal by default #3756

Merged
merged 5 commits into from
Mar 17, 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
7 changes: 7 additions & 0 deletions .changeset/tidy-pots-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'xstate': major
---

All transitions became internal by default. The style of the `target` pattern (`.child`, `sibling`, `#id`) has now no effect on the transition type.

Internal transitions don't reenter their source state when the target lies within it. You can still create external transitions (ones that reenter the source state under the mentioned circumstances) by explicitly setting `external: true` on the given transition.
3 changes: 2 additions & 1 deletion packages/core/src/StateNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export class StateNode<
source: this,
actions: this.initial.actions,
eventType: null as any,
external: false,
toJSON: () => ({
target: this.initial!.target!.map((t) => `#${t.id}`),
source: `#${this.id}`,
Expand Down Expand Up @@ -448,7 +449,7 @@ export class StateNode<
return !(
!transition.target &&
!transition.actions.length &&
transition.internal
!transition.external
);
})
.map((transition) => transition.eventType)
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/scxml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ function toConfig(
target: getTargets(targets),
...(value.elements ? executableContent(value.elements) : undefined),
...guardObject,
internal
external: !internal
};

if (eventType === NULL_EVENT) {
Expand Down
18 changes: 5 additions & 13 deletions packages/core/src/stateUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,17 +363,7 @@ export function formatTransition<
}
): AnyTransitionDefinition {
const normalizedTarget = normalizeTarget(transitionConfig.target);
const internal =
stateNode === stateNode.machine.root
? true // always internal for root
: 'internal' in transitionConfig
? transitionConfig.internal
: normalizedTarget
? normalizedTarget.some(
(_target) =>
isString(_target) && _target[0] === stateNode.machine.delimiter
)
: true;
const external = transitionConfig.external ?? false;
const { guards } = stateNode.machine.options;
const target = resolveTarget(stateNode, normalizedTarget);

Expand All @@ -394,7 +384,7 @@ export function formatTransition<
: undefined,
target,
source: stateNode,
internal,
external,
eventType: transitionConfig.event,
toJSON: () => ({
...transition,
Expand Down Expand Up @@ -537,6 +527,7 @@ export function formatInitialTransition<
source: stateNode,
actions: [],
eventType: null as any,
external: false,
target: resolvedTarget!,
toJSON: () => ({
...transition,
Expand Down Expand Up @@ -996,7 +987,7 @@ function getTransitionDomain(
}

if (
transition.internal &&
!transition.external &&
transition.source.type === 'compound' &&
targetStates.every((targetStateNode) =>
isDescendant(targetStateNode, transition.source)
Expand Down Expand Up @@ -1075,6 +1066,7 @@ export function microstep<
{
target: [...currentState.configuration].filter(isAtomicStateNode),
source: machine.root,
external: true,
actions: [],
eventType: null as any,
toJSON: null as any // TODO: fix
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ export interface TransitionConfig<
> {
guard?: GuardConfig<TContext, TExpressionEvent>;
actions?: BaseActions<TContext, TExpressionEvent, TEvent, TAction>;
internal?: boolean;
external?: boolean;
target?: TransitionTarget | undefined;
meta?: Record<string, any>;
description?: string;
Expand Down Expand Up @@ -1444,6 +1444,7 @@ export interface TransitionDefinition<
target: Array<StateNode<TContext, TEvent>> | undefined;
source: StateNode<TContext, TEvent>;
actions: BaseActionObject[];
external: boolean;
guard?: GuardDefinition<TContext, TEvent>;
eventType: TEvent['type'] | '*';
toJSON: () => {
Expand Down
12 changes: 6 additions & 6 deletions packages/core/test/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ describe('entry/exit actions', () => {
expect(called).toBe(true);
});

it('root entry/exit actions should not be called on root external transitions', () => {
it('root entry/exit actions should be called on root external transitions', () => {
let entrySpy = jest.fn();
let exitSpy = jest.fn();

Expand All @@ -559,7 +559,7 @@ describe('entry/exit actions', () => {
on: {
EVENT: {
target: '#two',
internal: false
external: true
}
},
initial: 'one',
Expand All @@ -578,8 +578,8 @@ describe('entry/exit actions', () => {

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

expect(entrySpy).not.toHaveBeenCalled();
expect(exitSpy).not.toHaveBeenCalled();
expect(entrySpy).toHaveBeenCalled();
expect(exitSpy).toHaveBeenCalled();
});

describe('should ignore same-parent state actions (sparse)', () => {
Expand Down Expand Up @@ -781,15 +781,15 @@ describe('entry/exit actions', () => {
]);
});

it('should enter all descendents when target is a descendent of current', () => {
it('should enter all descendents when target is a descendent of the source when using an external transition', () => {
const machine = createMachine({
initial: 'A',
states: {
A: {
initial: 'A1',
on: {
NEXT: {
internal: false,
external: true,
target: '.A2'
}
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/activities.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ describe('invocations (activities)', () => {
on: {
NEXT: {
target: 'a',
internal: false
external: true
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion packages/core/test/history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ describe('history states', () => {
states: {
a: {
on: {
REENTER: '#b_hist'
REENTER: {
target: '#b_hist',
external: true
}
},
initial: 'a1',
states: {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/internalTransitions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ const wordMachine = createMachine({
RIGHT_CLICK: '.right',
RIGHT_CLICK_EXTERNAL: {
target: '.right',
internal: false
external: true
},
CENTER_CLICK: '.center',
JUSTIFY_CLICK: '.justify',
RESET: 'direction', // explicit self-transition
RESET_TO_CENTER: {
target: 'direction.center',
internal: false
external: true
}
}
}
Expand Down
9 changes: 4 additions & 5 deletions packages/core/test/invoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,6 @@ describe('invoke', () => {
entry: () => entryActionsCount++,
on: {
UPDATE: {
internal: true,
actions: () => {
actionsCount++;
}
Expand Down Expand Up @@ -3252,8 +3251,8 @@ describe('invoke', () => {
service.send({ type: 'FINISH' });
expect(disposed).toBe(true);
});
// https://github.com/statelyai/xstate/issues/3072
it('root invocations should not restart on root external transitions', () => {

it('root invocations should restart on root external transitions', () => {
let count = 0;

const machine = createMachine({
Expand All @@ -3268,7 +3267,7 @@ describe('invoke', () => {
on: {
EVENT: {
target: '#two',
internal: false
external: true
}
},
initial: 'one',
Expand All @@ -3284,7 +3283,7 @@ describe('invoke', () => {

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

expect(count).toEqual(1);
expect(count).toEqual(2);
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ describe('json', () => {
"actions": [],
"event": "done.invoke.active:invocation[0]",
"eventType": "done.invoke.active:invocation[0]",
"external": false,
"guard": undefined,
"internal": false,
"source": "#active",
"target": [
"#(machine).foo",
Expand All @@ -157,8 +157,8 @@ describe('json', () => {
"actions": [],
"event": "error.platform.active:invocation[0]",
"eventType": "error.platform.active:invocation[0]",
"external": false,
"guard": undefined,
"internal": false,
"source": "#active",
"target": [
"#(machine).bar",
Expand All @@ -169,8 +169,8 @@ describe('json', () => {
"actions": [],
"event": "EVENT",
"eventType": "EVENT",
"external": false,
"guard": undefined,
"internal": false,
"source": "#active",
"target": [
"#(machine).foo",
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/predictableExec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ describe('predictableExec', () => {
on: {
REENTER: {
target: 'active',
internal: false
external: true
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const exampleMachine = createMachine<any, Events>({
on: {
EXTERNAL: {
target: 'one',
internal: false
external: true
},
INERT: {},
INTERNAL: {
Expand Down
2 changes: 1 addition & 1 deletion packages/xstate-inspect/test/inspect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ describe('@xstate/inspect', () => {
},
{
"sessionId": "x:7",
"state": "{"value":{},"done":false,"context":{"value":{"unsafe":"[unsafe]"}},"historyValue":{},"actions":[{"type":"xstate.assign","params":{"context":{"value":{"unsafe":"[unsafe]"}},"actions":[]}}],"event":{"type":"EV","value":{"unsafe":"[unsafe]"}},"_event":{"name":"EV","data":{"type":"EV","value":{"unsafe":"[unsafe]"}},"$$type":"scxml","type":"external"},"_sessionid":"x:7","_initial":false,"changed":true,"transitions":[{"actions":[{"type":"xstate.assign","params":{"assignment":{}}}],"event":"EV","source":"#(machine)","internal":true,"eventType":"EV"}],"children":{},"tags":[]}",
"state": "{"value":{},"done":false,"context":{"value":{"unsafe":"[unsafe]"}},"historyValue":{},"actions":[{"type":"xstate.assign","params":{"context":{"value":{"unsafe":"[unsafe]"}},"actions":[]}}],"event":{"type":"EV","value":{"unsafe":"[unsafe]"}},"_event":{"name":"EV","data":{"type":"EV","value":{"unsafe":"[unsafe]"}},"$$type":"scxml","type":"external"},"_sessionid":"x:7","_initial":false,"changed":true,"transitions":[{"actions":[{"type":"xstate.assign","params":{"assignment":{}}}],"event":"EV","source":"#(machine)","external":false,"eventType":"EV"}],"children":{},"tags":[]}",
"type": "service.state",
},
]
Expand Down
2 changes: 1 addition & 1 deletion packages/xstate-scxml/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function transitionToSCXML(
target: (transition.target || [])
.map((stateNode) => stateNode.id)
.join(' '),
type: transition.internal ? 'internal' : undefined
type: !transition.external ? 'internal' : undefined
}),
elements: elements.length ? elements : undefined
};
Expand Down
4 changes: 2 additions & 2 deletions packages/xstate-scxml/test/fixtures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe('toSCXML', () => {
encoding: 'utf-8'
});

const machine = require(`./fixtures/${testGroupName}/${testName}`)
.default;
const machine =
require(`./fixtures/${testGroupName}/${testName}`).default;

it(`${testGroupName}/${testName}`, () => {
expect(xml2js(toSCXML(machine))).toEqual(
Expand Down
8 changes: 6 additions & 2 deletions packages/xstate-scxml/test/fixtures/actionSend/send1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ export default createMachine({
on: {
t: {
target: '#b',
actions: actions.raise({ type: 's' })
actions: actions.raise({ type: 's' }),
external: true
}
}
},
b: {
id: 'b',
on: {
s: '#c'
s: {
target: '#c',
external: true
}
}
},
c: {
Expand Down
8 changes: 3 additions & 5 deletions packages/xstate-scxml/test/scxml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ describe('scxml', () => {

testGroupKeys.forEach((testGroupName) => {
testGroups[testGroupName].forEach((testName) => {
const originalMachine = require(`./fixtures/${testGroupName}/${testName}`)
.default;
const originalMachine =
require(`./fixtures/${testGroupName}/${testName}`).default;
const scxmlDefinition = toSCXML(originalMachine);

const scxmlTest = JSON.parse(
Expand Down Expand Up @@ -209,8 +209,7 @@ const lightMachine = createMachine({
on: {
TIMER: 'green',
POWER_OUTAGE: {
target: 'red',
internal: true
target: 'red'
}
},
...pedestrianStates
Expand Down Expand Up @@ -238,7 +237,6 @@ xdescribe('transition to SCXML', () => {
on: {
SOME_EVENT: {
target: 'next',
internal: true,
guard: () => true,
actions: ['foo', 'bar']
}
Expand Down