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] Higher-level guards #1456

Merged
merged 39 commits into from
Dec 5, 2020
Merged

[v5] Higher-level guards #1456

merged 39 commits into from
Dec 5, 2020

Conversation

davidkpiano
Copy link
Member

@davidkpiano davidkpiano commented Sep 12, 2020

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

🦋 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

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

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

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

🤷

@davidkpiano davidkpiano marked this pull request as ready for review October 21, 2020 02:24
guard: {
type: 'testGuard',
name: 'any',
children: [
Copy link
Member

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

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?

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants