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

Filter out transitions with undefined values from state.nextEvents. #59

Closed
wants to merge 1 commit into from

Conversation

threehams
Copy link

What

Don't include keys with undefined values in nextEvents.

Why

In lieu of #46, this allows configuration to be changed based on values which don't change after initial render. Example:

// 50% of people won't see the "confirmation" step:
const confirmationEnabled = useFeatureFlag("show-confirmation");

const [state, send] = useStateMachine()({
  states: {
    registration: {
      on: {
        NEXT: showConfirmation ? "confirmation" : undefined
      }
    }
    // other states
  }
})

This works with TypeScript, and sending "NEXT" correctly doesn't transition, but state.nextEvents shows ["NEXT"].
With this PR, state.nextEvents is [] if confirmationEnabled is false.

@cassiozen
Copy link
Owner

Hey @threehams - thanks for your PR.

I really like this, but we're currently working on a new beta version of this library (#56) and this code will need to be adapted to run there.

I'll ping you again when that is merged and we can work on this.

@threehams
Copy link
Author

Not a problem, I've been following the new branch and can send an updated PR once that's done.

@devanshj
Copy link
Contributor

devanshj commented Jul 26, 2021

On a gentle note: We might have to discourage or even disallow this kind of usage (meaning the definition not being a literal, here it's a union of two possibilities depending on showConfirmation) because of this. Cassio is yet to address it (maybe he missed the comment), let's see what happens.

I still haven't checked what happens if the definition is not literal but it seems it'd be really hard to support it.

@threehams
Copy link
Author

Until microsoft/TypeScript#13195 lands - which is an opt-in, very large breaking change - a union like "TOGGLE" | undefined is effectively the same as a missing one (with some subtleties). See if you hit one of those subtleties when your new branch stabilizes.

@devanshj
Copy link
Contributor

Sure thing. I think once it's available (I think it already is in 4.4?) we'll turn that on for this codebase (because more strict the better). Probably won't break stuff.

@cassiozen cassiozen closed this Sep 13, 2022
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.

3 participants