[work in progress] feature/xstate/angular

[ducin]

This is a very early implementation that already does its job. It's not ready for a merge yet, however, I'd like to start review with @Andarist asap so that it's easier later on. There might be some setup stuff (or some conventions?) I'm missing, most probably.

usage

export class AppComponent {
  #injector = inject(Injector);
  toggleActor = injectActor(toggleMachine, undefined, {
    injector: this.#injector
  });

  constructor() {
    this.toggleActor.ref.start();
  }

  handleToggleClick() {
    this.toggleActor.send({ type: 'toggle' });
  }
}

Nowadays, this is the standard approach in angular to get dependencies straight into a component (or another service):

  #injector = inject(Injector);
  toggleActor = injectActor(toggleMachine, undefined, {
    injector: this.#injector
  });

injecting the Injector looks odd, but that's actually expected, since there's a whole hierarchy of injectors, so you might get lots of different injectors when needed. And passing the injector via parameter is also a standard approach in libraries (either your call is within, so called, injection context and our adapter would get the injector itself, or one could pass it explicitly via param). All standard.

requirements

As @Andarist mentioned, there has to be a way to allow components having multiple state machines.

angular v17

The adpater exposes a signal-based API (the new Angular standard API for managing synchronous state).

interface InjectedActor<TLogic extends AnyActorLogic> {
  snapshot: Signal<SnapshotFrom<TLogic>>; // signal
  send: ActorRefFrom<TLogic>['send'];
  ref: ActorRefFrom<TLogic>;
}

It resembles react-based adapter's API.

cleanups

the most important part is actually this:
https://github.com/ducin/xstate/blob/feature/xstate/angular/packages/xstate-angular/src/injectActor.ts#L50-L53
all it does is it grabs destroyRef (an angular kinda-equivalent to useEffect return-callback) from a certain injector. Also, pretty standard.

questions

• wouldn't it make sense for the adapter to automatically start() the machine? Wouldn't it be the most common scenario to immediately start the machine (without the need to do that explicitly)?

further work

I want to finish the following before considering the PR ready:

• cleanups (e.g. comments), obviously
• some more examples (working on it)
• analyzing possibility to provide globally available machines (could be achieved by explicitly creating a service and making it available in certain areas of application, so it's an optional feature)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 504c296

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ducin]

@Andarist what should we do with this?
Angular v17 requires node v18+. Going back to v16 makes little sense for the adapter since there's no stable signal support, and without the signal support the adaption would be significantly smaller (it's the new standard).

error @angular/common@17.0.7: The engine "node" is incompatible with this module. Expected version "^18.13.0 || >=20.9.0". Got "16.18.1"
error Found incompatible module.

[Andarist]

@Andarist what should we do with this?

This might have already been addressed in the meantime. Could you sync with main? Its CI is already using node 20

[Andarist]

Shouldn't this dependency list mention @xstate/angular too? It has not been published yet though and examples are outside of the core monorepo... so this might be slightly problematic right now. Maybe it would be worth to split this (before the merge) into 2 different PRs? One could introduce @xstate/angular and the other one could introduce an example using it.

[Andarist]

those comments here should be cleaned up before merge

[Andarist]

If we split this PR into two (like mentioned in the other comment) then we could remove this, right? Then @xstate/angular would just get installed from the registry and things would "just work"

[Andarist]

It seems that this PR introduces 2 different lockfiles in this example - we should stick to one 😉 I think most of our examples are meant to be using pnpm - but I see that some of them might have some leftover yarn/npm lockfiles too.

[Andarist]

I removed the mentioned leftovers here: #4641

[Andarist]

this likely shouldnt be touched as part of this PR 😉

[Andarist]

I'd probably remove this file altogether. It will just get auto-created in the future when we release new versions of this package.

[Andarist]

It would be great to have at least a basic usage example in this README

[Andarist]

All other packages mention David as the author so it feels weird to have a different author here. But I don't know, an "author" field in the MIT-licensed OSS software is a weird concept to me 🤷‍♂️ Maybe we should just remove this field altogether from (almost?) all packages?

[Andarist]

is this a requirement? It certainly makes the current exports configuration incorrect since now .js files will be treated as modules but they are actually scripts

[Andarist]

If possible - I would prefer to remove this field from here.

[Andarist]

If we still have this in other packages we should remove this in another PR

Suggested change

	"peerDependenciesMeta": {
	"xstate": {
	"optional": true
	}
	},

[Andarist]

are all of those required as peer deps? :o it's kinda crazy 😅

[Andarist]

do we use anything beyond @angular/core here? I understand that a lot of this might be required in the final app - but are those anyhow related to this standalone~ library? 🤔

[Andarist]

If we don't use #is-development for anything within this package then let's remove this (and the associated configuration from the package.json)

[Andarist]

What would this potential export do? Is this related to the global actors idea that you have mentioned in the description of this PR?

[Andarist]

Suggested change

// result.snapshot = snapshot;

[Andarist]

While this comment might be relevant to a problem that you have faced it feels out of place here. Did you end up using any of the workarounds mentioned here?

[Andarist]

If you sync with main you should be able to remove this cast:

Suggested change

	const actorInstance = createActor(logic as any, options).start();
	const actorInstance = createActor(logic, options).start();

[Andarist]

I'd probably merge those two:

Suggested change

	options?: ActorOptions<TLogic>,
	injectOptions?: injectActorOptions
	{ injector = inject(Injector), ...options }: ActorOptions<TLogic> & { injector?: Injector } = {},

[Andarist]

is it a common practice to use the default injector like this?

[Andarist]

I definitely like that this is an option and that people won't have to always pass injector in explicitly

[Andarist]

wouldn't it make sense for the adapter to automatically start() the machine? Wouldn't it be the most common scenario to immediately start the machine (without the need to do that explicitly)?

Yes. That would be best, in my opinion.

I also wonder, what's the story around SSR in Angular? Is it enough to put .start() in OnInit to have this compatible with SSR?

[Andarist]

I usually find it much less error-prone to dispose of local things first before stopping the external~ resource. It usually leads to removing some edge cases around synchronous notification from within the stopping call or ones related to the fact that the other call might throw and interrupt the whole cleanup. I don't think we have to wrap this in any try/catches or nothing - nothing here should be able to throw (if it does then I'd consider that to be a likely bug in XState). It's just a rule of thumb that I developed over the years and try to follow as much as possible

Suggested change

	actorInstance.stop();
	subscription.unsubscribe();
	subscription.unsubscribe();
	actorInstance.stop();