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

[core] Modeling events with createModel() #1955

Merged
merged 15 commits into from
Mar 17, 2021
Merged

Conversation

davidkpiano
Copy link
Member

Making this PR to discuss and see how we can improve this. This PR provides type inference for events in createModel() that are specified as "event creators":

const userModel = createModel(
  {
    name: 'David',
    age: 30
  },
  {
    updateName: (value: string) => ({ value }),
    updateAge: (value: number) => ({ value }),
    anotherEvent: () => ({})
  }
);

// Example of an externally-defined assign action
const assignName = userModel.assign(
  {
    name: (_, event) => {
      return event.value;
    }
  },
  'updateName'
);

const machine = createMachine<
  typeof userModel['types']['context'],
  typeof userModel['types']['events']
>({
  context: userModel.initialContext,
  initial: 'active',
  states: {
    active: {
      on: {
        updateName: {
          actions: assignName
        },
        updateAge: {
          actions: userModel.assign((_, e) => {
            return {
              age: e.value
            };
          })
        }
      }
    }
  }
});

let updatedState = machine.transition(
  undefined,
  userModel.events.updateName('Anyone')
);

expect(updatedState.context.name).toEqual('Anyone');

updatedState = machine.transition(
  updatedState,
  userModel.events.updateAge(42)
);

expect(updatedState.context.age).toEqual(42);

This is a little awkward, sure:

const machine = createMachine<
  typeof userModel['types']['context'],
  typeof userModel['types']['events']
>({/* ... */})

But the important part is this:

let updatedState = machine.transition(
  undefined,
  userModel.events.updateName('Anyone')
);

updatedState = machine.transition(
  updatedState,
  userModel.events.updateAge(42)
);

The second argument to createModel(initialCtx, eventCreators) enables two things:

  • Events to be easily created and strongly typed using these convenient event creators, and...
  • Event types to be inferred automatically from these event creators! 🎉

Let me know your thoughts!

@changeset-bot
Copy link

changeset-bot bot commented Feb 23, 2021

🦋 Changeset detected

Latest commit: 5febfe8

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

This PR includes changesets to release 3 packages
Name Type
xstate Minor

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

@HipsterBrown
Copy link

Instead of needing to do:

const machine = createMachine<typeof myModel['types']['context'], typeof myModel['types']['events']>({});

Could there be a function definition for createMachine to accept a ContextModel type argument that uses those types fields?

export function createMachine<ContextModel, TTypeStates>(config: MachineConfig): StateMachine<ContextModel['types']['context'], any, ContextModel['types']['events'], TTypeStates>;

@davidkpiano
Copy link
Member Author

Instead of needing to do:

const machine = createMachine<typeof myModel['types']['context'], typeof myModel['types']['events']>({});

Could there be a function definition for createMachine to accept a ContextModel type argument that uses those types fields?

export function createMachine<ContextModel, TTypeStates>(config: MachineConfig): StateMachine<ContextModel['types']['context'], any, ContextModel['types']['events'], TTypeStates>;

Ideally yes, but I don't know if it's possible to have function overloading with different generic types... @Andarist @mattpocock ?

@HipsterBrown
Copy link

I don't know if it's possible to have function overloading with different generic types

It looks like the Machine function does this currently: https://github.com/davidkpiano/xstate/blob/master/packages/core/src/Machine.ts#L13-L29

@Andarist
Copy link
Member

Ideally yes, but I don't know if it's possible to have function overloading with different generic types... @Andarist @mattpocock ?

Yep, totally possible.

@eddiewang
Copy link

looks great! I have been using createModel in my machines lately to make typing more consistent and it's been working well. would this be compatible with the createUpdater fns from Immer?

@davidkpiano
Copy link
Member Author

looks great! I have been using createModel in my machines lately to make typing more consistent and it's been working well. would this be compatible with the createUpdater fns from Immer?

It should be! I haven't tested it out, might need to update the types or create an Immer-specific createModel.

@cbmd
Copy link

cbmd commented Mar 5, 2021

@davidkpiano how about to implement an array of events acceptable?
Something like....

import isType from '@sindresorhus/is';
import type { EventObject } from 'xstate';
import type { AssertedTypesUnion } from './types';

export function assertEventType<
  TEvent extends EventObject,
  TExpectedType extends TEventTypeArray | TEventType,
  TEventType extends TEvent['type'],
  TEventTypeArray extends TEventType[]
>(
  event: TEvent,
  expected: TExpectedType,
): asserts event is TEvent & {
  type: AssertedTypesUnion<TExpectedType>;
} {
  const events = isType.array<TExpectedType>(expected) ? expected : [expected];
  const { type } = event;
  if (!events.includes(type as TExpectedType)) {
    throw new Error(
      `Invalid event: expected one of "${events.join(
        ',',
      )}" but got "${type}" instead.`,
    );
  }
}

wherever isType from @sindresorhus/is surely could be eliminated, just copy/pasted from existing project to make it simple...
Any objections against adding that @davidkpiano / @Andarist?

packages/core/src/model.ts Outdated Show resolved Hide resolved
packages/core/test/model.test.ts Outdated Show resolved Hide resolved
packages/core/src/model.ts Outdated Show resolved Hide resolved
packages/core/src/model.ts Outdated Show resolved Hide resolved
packages/core/src/model.ts Outdated Show resolved Hide resolved
packages/core/src/model.ts Outdated Show resolved Hide resolved
packages/core/src/model.ts Show resolved Hide resolved
packages/core/src/model.ts Outdated Show resolved Hide resolved
@davidkpiano davidkpiano marked this pull request as ready for review March 16, 2021 00:10
@davidkpiano davidkpiano merged commit d1ae686 into master Mar 17, 2021
@davidkpiano davidkpiano deleted the davidkpiano/model-events branch March 17, 2021 13:43
@github-actions github-actions bot mentioned this pull request Mar 17, 2021
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.

None yet

5 participants