cassiozen / useStateMachine

The <1 kb state machine hook for React
MIT License
2.38k stars 47 forks source link

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

Closed threehams closed 1 year ago

threehams commented 3 years ago

What

Don't include keys with undefined values in nextEvents.

Why

In lieu of https://github.com/cassiozen/useStateMachine/issues/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 commented 3 years ago

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 commented 3 years ago

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

devanshj commented 3 years ago

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 commented 3 years ago

Until https://github.com/microsoft/TypeScript/issues/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 commented 3 years ago

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.