Required input when defined inside types

[SandroMaglione]

This PR updates the types to require input when defined inside types/input when using useActor, useMachine, and useActorRef.

Rationale

With the current types even if types/input is defined, when using hooks like useActor the input option remains optional.

In my opinion this is an undesired behaviour, since it should be safe to assume that if the user defines an input it's expected to be provided when using the actor.

More than once I encountered runtime issues caused by forgetting to pass the input.

Solution

This PR copies the implementation of createActor that requires to provide input when defined.

---

Issue with useActorRef

It's probably not possible to make the same update to useActorRef for react and vue since the type is made optional by spreading options as the last function argument, but useActorRef has a third parameter observerOrListener, which does not allow to spread options to make them optional:

xstate/packages/core/src/createActor.ts

Lines 792 to 799 in 1ab9745

	...[options]: ConditionalRequired<
	[
	options?: ActorOptions<TLogic> & {
	[K in RequiredOptions<TLogic>]: unknown;
	}
	],
	IsNotNever<RequiredOptions<TLogic>>
	>

xstate/packages/xstate-react/src/useActorRef.ts

Lines 45 to 51 in 1ab9745

	export function useActorRef<TLogic extends AnyActorLogic>(
	machine: TLogic,
	options: ActorOptions<TLogic> = {},
	observerOrListener?:
	| Observer<SnapshotFrom<TLogic>>
	| ((value: SnapshotFrom<TLogic>) => void)
	): Actor<TLogic> {

I opted instead to make input required while options is still optional. This makes input required when passing options, but if options is not passed the type will work even if types/input is defined:

xstate/packages/xstate-react/src/useActorRef.ts

Lines 55 to 68 in 10ed048

	export function useActorRef<TLogic extends AnyActorLogic>(
	machine: TLogic,
	options?: ConditionalRequired<
	[
	options?: ActorOptions<TLogic> & {
	[K in RequiredOptions<TLogic>]: unknown;
	}
	],
	IsNotNever<RequiredOptions<TLogic>>
	>['0'],
	observerOrListener?:
	| Observer<SnapshotFrom<TLogic>>
	| ((value: SnapshotFrom<TLogic>) => void)
	): Actor<TLogic> {

A solution may be to merge observerOrListener inside options. This may be a breaking change.

[changeset-bot]

🦋 Changeset detected

Latest commit: 08932cd

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

This PR includes changesets to release 5 packages

Name	Type
xstate	Patch
@xstate/svelte	Patch
@xstate/react	Patch
@xstate/solid	Patch
@xstate/vue	Patch

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

[Andarist]

this type should get renamed to be more specific if we want to export it

[SandroMaglione]

export type RequiredOptions<TLogic extends AnyActorLogic> =
  undefined extends InputFrom<TLogic> ? never : 'input';

Something like ActorHasRequiredInputOption?

[Andarist]

This one returns a union of required options - not a boolean. So I'd go with RequiredActorInstantiationOptions or smth like this.

[SandroMaglione]

Opted for RequiredActorInstanceOptions

[Andarist]

It's probably not possible to make the same update to useActorRef for react and vue since the type is made optional by spreading options as the last function argument, but useActorRef has a third parameter observerOrListener, which does not allow to spread options to make them optional:

You can create [options?, observerOrListener?], make the first element required conditionally and spread the result. This way you will make the second argument conditionally required while keeping the third one optional

[SandroMaglione]

You can create [options?, observerOrListener?], make the first element required conditionally and spread the result. This way you will make the second argument conditionally required while keeping the third one optional

@Andarist not sure this works. I tried various implementations, but the issue is that making a parameter conditionally required requires to spread it, but spread is allowed only for the last parameter.

This below doesn’t work A rest element must be last in a destructuring pattern.ts(2462):

export function useActorRef<TLogic extends AnyActorLogic>(
  machine: TLogic,
  ...[...[options], observerOrListener]: [
    options: ConditionalRequired<
      [
        options?: ActorOptions<TLogic> & {
          [K in RequiredOptions<TLogic>]: unknown;
        }
      ],
      IsNotNever<RequiredOptions<TLogic>>
    >,
    observerOrListener?:
      | Observer<SnapshotFrom<TLogic>>
      | ((value: SnapshotFrom<TLogic>) => void)
  ]
): Actor<TLogic>

These two solutions below make the parameter always required:

export function useActorRef<TLogic extends AnyActorLogic>(
  machine: TLogic,
  ...[options, observerOrListener]: [
    options: ConditionalRequired<
      [
        options?: ActorOptions<TLogic> & {
          [K in RequiredOptions<TLogic>]: unknown;
        }
      ],
      IsNotNever<RequiredOptions<TLogic>>
    >,
    observerOrListener?:
      | Observer<SnapshotFrom<TLogic>>
      | ((value: SnapshotFrom<TLogic>) => void)
  ]
): Actor<TLogic>

export function useActorRef<TLogic extends AnyActorLogic>(
  machine: TLogic,
  ...[options, observerOrListener]: [
    ConditionalRequired<
      [
        options?: ActorOptions<TLogic> & {
          [K in RequiredOptions<TLogic>]: unknown;
        }
      ],
      IsNotNever<RequiredOptions<TLogic>>
    >['0'],
    observerOrListener?:
      | Observer<SnapshotFrom<TLogic>>
      | ((value: SnapshotFrom<TLogic>) => void)
  ]
): Actor<TLogic>

I also tried other options without much success. What do you think?

[Andarist]

Those type helpers are not too readable anyway today so I wouldn't try to reuse them at all costs. At the very least something along those lines should work:

export function useActorRef<TLogic extends AnyActorLogic>(
  machine: TLogic,
  ...[options, observerOrListener]: IsNotNever<
    RequiredOptions<TLogic>
  > extends true
    ? [
        options: ActorOptions<TLogic> & {
          [K in RequiredOptions<TLogic>]: unknown;
        },
        observerOrListener?:
          | Observer<SnapshotFrom<TLogic>>
          | ((value: SnapshotFrom<TLogic>) => void),
      ]
    : [
        options?: ActorOptions<TLogic>,
        observerOrListener?:
          | Observer<SnapshotFrom<TLogic>>
          | ((value: SnapshotFrom<TLogic>) => void),
      ]
): Actor<TLogic>;

I don't mind some mild repetition.

[Andarist]

LGTM but before landing this, type-level tests should be added. (and it requires proper changesets)

[SandroMaglione]

@Andarist added type tests for required input and changeset. I marked this as a patch release, since I would consider this a bug fix. Let me know if this works.

[Andarist]

LGTM - I just left one comment to address

[pckilgore]

My useMachine calls requires an empty object as a second parameter after this PR, even if there is no input required in any actors. Is that just the price I pay here?

[Andarist]

@pckilgore please provide a repro case so we can take a look at your case

[SandroMaglione]

@pckilgore if your input is in the form { input: { value?: string; }} it will result required since only value is optional, whereas input is required to be defined

[pckilgore]

I was able to fix by updating xstate itself to 5.19 from 5.18. Yarn didn't give me any peer dependency warnings about the mismatch, which isn't what I would have expected?

Sorry to bother.

[pckilgore]

@pckilgore if your input is in the form { input: { value?: string; }} it will result required since only value is optional, whereas input is required to be defined

That would have still been odd, because I would have expected typescript to accept { input: {} } in that circumstance, not just {} as the second parameter. Appreciate the suggestion!

[SandroMaglione]

Yes, I would expect { input: {} } as well. Was it working with just {}? If so I wouldn’t be sure, definitely a repro is needed