Skip to content

Commit

Permalink
feat(no-infinite-loop): add detection of incorrect eventless transiti…
Browse files Browse the repository at this point in the history
…ons in xstate v5
  • Loading branch information
rlaffers committed Aug 16, 2023
1 parent 15f778c commit 360b609
Show file tree
Hide file tree
Showing 3 changed files with 319 additions and 39 deletions.
89 changes: 88 additions & 1 deletion docs/rules/no-infinite-loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ createMachine({
// ❌ The action on the 2nd transition does not update the context,
// so executing it will not make the 1st transition valid on the next evaluation.
// (or the 1st transition is always taken, so the 2nd transition is useless)
// XState v4
createMachine({
states: {
deciding: {
Expand All @@ -60,6 +61,24 @@ createMachine({
},
})

// ❌ Same as above with XState v5
createMachine({
states: {
deciding: {
always: [
{
guard: ({ context }) => context.count > 5,
target: 'idle',
},
// no guard, no target, no assign action
{
actions: () => console.log('hello'),
},
],
},
},
})

// ❌ No guard, no target. Even though it updates the context,
// it is the first transition in sequence, so it will be taken ad infinitum.
createMachine({
Expand Down Expand Up @@ -90,6 +109,7 @@ createMachine({
// ❌ No target. The action does not update the context. This transition is
// either useless (never taken because of its guard), or guarantees an
// infinite loop error (if its guard is passed once then it will always be passed).
// XState v4
createMachine({
states: {
deciding: {
Expand All @@ -102,12 +122,26 @@ createMachine({
},
},
})

// ❌ Same as above with XState v5
createMachine({
states: {
deciding: {
always: [
{
guard: () => {},
actions: () => console.log('hello'),
},
],
},
},
})
```

Examples of **correct** code for this rule:

```javascript
// ✅ Has target
// ✅ Has target (XState v4)
createMachine({
states: {
deciding: {
Expand All @@ -124,8 +158,26 @@ createMachine({
},
})

// ✅ Has target (XState v5)
createMachine({
states: {
deciding: {
always: [
{
guard: () => {},
target: 'busy',
},
{
target: 'idle',
},
],
},
},
})

// ✅ The second transition updates the context, so there's a chance
// that the first transition will eventually become valid.
// XState v4
createMachine({
states: {
deciding: {
Expand All @@ -142,8 +194,26 @@ createMachine({
},
})

// ✅ XState v5
createMachine({
states: {
deciding: {
always: [
{
guard: ({ context }) => context.count > 5,
target: 'idle',
},
{
actions: assign({ count: ({ context }) => context.count + 1 }),
},
],
},
},
})

// ✅ We are optimistic about the 2nd transition action updating the context.
// Therefore there's a chance that the first transition will eventually become valid.
// XState v4
createMachine({
states: {
deciding: {
Expand All @@ -159,4 +229,21 @@ createMachine({
},
},
})

// XState v5
createMachine({
states: {
deciding: {
always: [
{
guard: ({ context }) => context.count > 5,
target: 'idle',
},
{
actions: 'someAction', // hopefully an assign() action
},
],
},
},
})
```
57 changes: 39 additions & 18 deletions lib/rules/no-infinite-loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const {
isFunctionExpression,
isStringLiteralOrIdentifier,
} = require('../utils/predicates')
const getSettings = require('../utils/getSettings')

function isEventlessTransitionDeclaration(node) {
return node.type === 'Property' && node.key.name === 'always'
Expand Down Expand Up @@ -56,6 +57,11 @@ function isTransitionToItself(transition) {
return targetStateNodeNameOrID === parentStateNodeNameOrID
}

function isConditionalTransition(node, version) {
const guardPropName = version === 4 ? 'cond' : 'guard'
return hasProperty(guardPropName, node)
}

function isAssignCall(node) {
return node.type === 'CallExpression' && node.callee.name === 'assign'
}
Expand Down Expand Up @@ -90,25 +96,33 @@ function actionsContainStringLiteralOrIdentifier(transition) {
)
}

function getGuard(transition) {
if (!hasProperty('cond', transition)) {
const getGuard = (version) => (transition) => {
const guardPropName = version === 4 ? 'cond' : 'guard'
if (!hasProperty(guardPropName, transition)) {
return Nothing
}
const guard = transition.properties.find(propertyHasName('cond')).value
const guard = transition.properties.find(propertyHasName(guardPropName)).value
if (!guard) {
return Nothing
}
return Just(guard)
}

const guardUsesContext = (guardFunctionExpression) =>
guardFunctionExpression.params.length > 0
const guardUsesContext = (version) => (guardFunctionExpression) =>
version === 4
? guardFunctionExpression.params.length > 0
: guardFunctionExpression.params[0]?.type === 'ObjectPattern' &&
guardFunctionExpression.params[0].properties.some(
propertyHasName('context')
)

const guardIsFunctionAndNotCheckingContext = pipe([
getGuard,
map(allPass([isFunctionExpression, complement(guardUsesContext)])),
fromMaybe(false),
])
function guardIsFunctionAndNotCheckingContext(node, version) {
return pipe([
getGuard(version),
map(allPass([isFunctionExpression, complement(guardUsesContext(version))])),
fromMaybe(false),
])(node)
}

module.exports = {
meta: {
Expand Down Expand Up @@ -145,14 +159,18 @@ module.exports = {
},

create: function (context) {
const { version } = getSettings(context)
return {
// always: {}
// always: { actions: whatever}
'Property[key.name="always"] > ObjectExpression': function (node) {
if (!isInsideMachineDeclaration(node)) {
return
}
if (!hasProperty('target', node) && !hasProperty('cond', node)) {
if (
!hasProperty('target', node) &&
!isConditionalTransition(node, version)
) {
context.report({
node,
messageId: 'noTargetNoGuardIsSingle',
Expand All @@ -167,7 +185,10 @@ module.exports = {
if (!isInsideMachineDeclaration(node)) {
return
}
if (!hasProperty('target', node) && !hasProperty('cond', node)) {
if (
!hasProperty('target', node) &&
!isConditionalTransition(node, version)
) {
if (isFirstArrayItem(node)) {
context.report({
node,
Expand Down Expand Up @@ -214,7 +235,7 @@ module.exports = {
// always: { target: 'idle' }
// }]
const targetsItself = isTransitionToItself(node)
if (!hasProperty('cond', node) && targetsItself) {
if (!isConditionalTransition(node, version) && targetsItself) {
if (isFirstArrayItem(node)) {
context.report({
node,
Expand All @@ -232,7 +253,7 @@ module.exports = {
}

if (
hasProperty('cond', node) &&
isConditionalTransition(node, version) &&
targetsItself &&
!hasAssignAction(node) &&
!actionsContainStringLiteralOrIdentifier(node)
Expand All @@ -245,9 +266,9 @@ module.exports = {
}

if (
hasProperty('cond', node) &&
isConditionalTransition(node, version) &&
targetsItself &&
guardIsFunctionAndNotCheckingContext(node) &&
guardIsFunctionAndNotCheckingContext(node, version) &&
isFirstArrayItem(node)
) {
context.report({
Expand All @@ -260,7 +281,7 @@ module.exports = {

// always: [{ cond: something, actions: something }]
if (
hasProperty('cond', node) &&
isConditionalTransition(node, version) &&
hasProperty('actions', node) &&
!hasProperty('target', node)
) {
Expand All @@ -280,7 +301,7 @@ module.exports = {
// or an infinite loop.
if (
isFirstArrayItem(node) &&
guardIsFunctionAndNotCheckingContext(node)
guardIsFunctionAndNotCheckingContext(node, version)
) {
context.report({
node,
Expand Down
Loading

0 comments on commit 360b609

Please sign in to comment.