Skip to content

[v5] Higher-level guards#1456

Merged
davidkpiano merged 39 commits intonextfrom
v5/guards
Dec 5, 2020
Merged

[v5] Higher-level guards#1456
davidkpiano merged 39 commits intonextfrom
v5/guards

Conversation

@davidkpiano
Copy link
Member

@davidkpiano davidkpiano commented Sep 12, 2020 •
edited
Loading

This PR aims to simplify the implementation details and TS types for guards, as well as adds the following higher-level guards:

  • and([someGuard, anotherGuard, ...])
  • or([someGuard, anotherGuard, ...])
  • not(someGuard)
  • Others?

These higher-level guards can be combined at any depth:

on: {
  EVENT: {
    target: 'somewhere',
    guard: and(['stringGuard', or([{ type: 'anotherGuard' }, not(() => false)])])
  }
}

And parameterized guards can use these higher-level guards:

on: {
  EVENT: { target: 'somewhere', guard: 'everythingValid' }
},

// ...

// machine options
guards: {
  thisValid: () => true,
  thatValid: not('thatInvalid'),
  thatInvalid: (context, event) => false,
  everythingValid: and(['thisValid', 'thatValid'])
}

It also will rename cond to guard in transitions, as this has been a common source of confusion.

TODO:

  • Add and([...]) + tests
  • Add or([...]) + tests
  • Add not(...) + tests
  • Add other boolean guards (if applicable)
  • Enable "guard definitions" in guards instead of just predicates (not in this PR)
  • Rename cond to guards
  • Simplify types

@changeset-bot
Copy link

changeset-bot bot commented Sep 12, 2020 •
edited
Loading

🦋 Changeset detected

Latest commit: 8fcbddd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
xstate Major
@xstate/graph Major
@xstate/immer Major
@xstate/inspect Major
@xstate/react Major
@xstate/test Major
@xstate/vue Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 12, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0fe1e18:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

@johnyanarella
Copy link

johnyanarella commented Sep 19, 2020 •
edited
Loading

Really like this idea. Captures more interesting visualizable intent, too!

Given the variadic signature, wondering if every() (or all()), some() (or any()), etc. might read better left to right? That said not() reads better than none() would.

@johnyanarella
Copy link

johnyanarella commented Sep 19, 2020

After further thought, now I've convinced myself you had it right with and and or.

🤷

Andarist
Andarist reviewed Sep 21, 2020
View reviewed changes
Andarist
Andarist reviewed Sep 21, 2020
View reviewed changes
Andarist
Andarist reviewed Sep 21, 2020
View reviewed changes
@davidkpiano davidkpiano marked this pull request as ready for review October 21, 2020 02:24
Andarist
Andarist reviewed Nov 7, 2020
View reviewed changes
packages/core/test/guards.test.ts
guard: {
type: 'testGuard',
name: 'any',
children: [
Copy link
Collaborator

@Andarist Andarist Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super convinced that having a custom predicate that is responsible for resolving its children is a good idea - if u need something like this you can already use params, shouldn't children just be reserved for combinators like not, and and or?

In other words - could u prepare a real-life example of such a thing used in practice?

Copy link
Member Author

@davidkpiano davidkpiano Nov 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if u need something like this you can already use params

Not exactly, since params are just params - they're not resolved. children are resolved.

The usefulness of this is so that custom guards that might need to reference other guards can be made in userland (such as xor(...), etc.).

@cbmd
Copy link

cbmd commented Nov 9, 2020

@davidkpiano guards not included in package.json in this PR. Is that intentional?

Andarist
Andarist approved these changes Dec 4, 2020
View reviewed changes
@davidkpiano davidkpiano merged commit f1ee38d into next Dec 5, 2020
@davidkpiano davidkpiano deleted the v5/guards branch December 5, 2020 14:49
@github-actions github-actions bot mentioned this pull request Dec 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Andarist Andarist Andarist approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@davidkpiano @johnyanarella @cbmd @Andarist