statelyai / xstate

Actor-based state management & orchestration for complex app logic.
https://stately.ai/docs
MIT License
26.98k stars 1.24k forks source link

First Draft for @xstate/angular #4816

Open niklas-wortmann opened 6 months ago

niklas-wortmann commented 6 months ago

This is a first draft for an angular implementation for XState.

As you will notice, it does need some tests, and also for convenience, we could add an angular add schematic, but other than that it is working in the tests I ran.

It did take heavy inspiration from the xstate/vue implementation but also ngrx signalstore.

Let me know what you think!

changeset-bot[bot] commented 6 months ago

⚠️ No Changeset found

Latest commit: 4ad82d13f0eb92d5ec390fcb7bef28d02d6d710c

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

niklas-wortmann commented 6 months ago

Just saw that the pipeline is failing, and I also did some copy paste mistakes in the package Json, I will fix those tomorrow morning

Andarist commented 6 months ago

I wonder if you had a chance to take a look at https://github.com/statelyai/xstate/pull/4638 . How would you compare both approaches?

niklas-wortmann commented 6 months ago

@Andarist overall the implementation is pretty similar. I am slightly biased here, but I think my implementation is a little more ergonomic and flexible, also I prefer not to expose the user to deal with the injection context. My implementation abstracts this better. Also, this approach provides more flexibility in the way the service is provided, as a service can extend useActor to add additional logic. But as I said, I might be biased here and would appreciate other opinions, maybe @ducin has an opinion on this

niklas-wortmann commented 6 months ago

What is the best way forward with this? I'd be more than happy to add tests and schematics if this is of interest (probably in a separate pr though). Also is there anything I can do about the ci/codesandbox issue?

Andarist commented 6 months ago

Tests should be added here - as part of this PR. I'm not sure what do you mean by schematics.

Also is there anything I can do about the ci/codesandbox issue?

I wouldn't be worried about CodeSandbox. It's not a blocker - but we should bump the node version used by that tool. This can be done here but:

It might be good to schedule that call to talk through the build intricacies if you are still up for it. I would mainly like to just get myself up to speed with the angular ways of doing things ;p

niklas-wortmann commented 6 months ago

@Andarist Alrighty I added a couple of tests, let me know what you think. I did test the implementation in an SSR scenario and it worked just fine with the current implementation 🎉

I also checked in with the other Angular GDEs about the ng-packagr. It seems like ng-packagr mostly ensures efficient builds with the angular cli and the guarantee that it works with the bundler. But as Angular Package Format is just a specification it is not a set requirement to use it. So I guess you can try to move forward with preconstruct and I can help test builds if you want.

niklas-wortmann commented 6 months ago

I also noticed that the build is now failing, it doesn't really seem to be related to any of my changes unless I am missing something?

Andarist commented 6 months ago

I also noticed that the build is now failing, it doesn't really seem to be related to any of my changes unless I am missing something?

It seems that you have forcefully updated the lockfile or something. I've fixed it with git checkout origin/main yarn.lock && yarn

I did test the implementation in an SSR scenario and it worked just fine with the current implementation 🎉

I suspect it still might be a problem. Could you check what happens with a machine like this?

createMachine({
  entry: () => { throw new Error('oops') }
})

This error should only be thrown on the client side.

So I guess you can try to move forward with preconstruct and I can help test builds if you want.

I'll try to do that 👍

niklas-wortmann commented 6 months ago

You were right the error was thrown on the server, but that is now fixed!

niklas-wortmann commented 6 months ago

The failing test actually made me aware of a behavior that I did not necessarily expect. With the change I did to handle the SSR case, now a machine needs to be injected into a component to be started, this means if you have a global machine but it isn't used in a component yet state transitions just get lost. This doesn't sound like the correct behavior to me. @Andarist can you elaborate on the original issue you had in the SSR case?

Andarist commented 6 months ago

this means if you have a global machine but it isn't used in a component yet state transitions just get lost.

Could you elaborate on what happens in this scenario?

@Andarist can you elaborate on the original issue you had in the SSR case?

In every other framework actors are not meant to be started at the server. Each SSR render (yes, ATM machine) is meant to be scoped to the current request. If an actor gets started on the server then it might execute some side-effects and SSR is meant to be pure-ish when it comes to what happens in the components to avoid leaks between requests and stuff like that. Also, we need a "symmetry" - if an actor gets started during SSR... what about the cleanup?

niklas-wortmann commented 5 months ago

Could you elaborate on what happens in this scenario?

So the machine will be started if it is injected in a component that is getting rendered. If this is not the case because it is a global state that is not rendered somewhere yet, every state transition will get lost.

Here's an example that makes it more tangible

const counterMachine = createMachine({
  initial: 'active',
  context: {
    count: 0
  },
  states: { active: {
      on: {
        INC: {
          actions: assign({
            count: (context: any) => {
              return context.context.count + 1;
            }
          })
        },
        DEC: {
          actions: assign({
            count: (context: any) => {
              return context.context.count - 1;
            }
          })
        }
      }
    }}
});

const CounterMachine = useMachine(counterMachine, {providedIn: "root"})

@Component({
  selector: 'app-test',
  standalone: true,
  providers: [CounterMachine],
  template: `
    <button (click)="counterMachine.send({ type: 'DEC' })"

    >Decrement</button>
    {{counterMachine.snapshot().context.count }}
    <button (click)="counterMachine.send({ type: 'INC' })">Increment</button>

  `,
  styleUrl: './test.component.css'
})
export class TestComponent {
  protected counterMachine = inject(CounterMachine)
}

@Injectable({providedIn: 'root'})
export class Service {

  constructor() {
    const machine = inject(CounterMachine);
    machine.send({ type: 'INC' }); // THIS IS NOT HAVING ANY EFFECT AS THE MACHINE IS NOT STARTED YET
  }
}

I'd also assume that you easily get hydration mismatch errors if the machine is not started on the server but on the client (not super familiar with SSR, so I might be missing something here)

Andarist commented 5 months ago

So I think what you are saying is that transitions won't get executed on the server side (because the machine is not started yet) and that can lead to a hydration mismatch. I mean, I'm just repeating what you said - I just want to make sure that you are talking about a single problem (I see those 2 as basically the same problem).

You are right but I think this is expected. If you want to avoid a hydration mismatch then you need to "stabilize" your initial value that gets SSRed - and not rely on transitions being executed there to achieve that.

niklas-wortmann commented 5 months ago

Sorry I could have expressed this better, but I think those are 2 separate issues (at least for me) Regarding the mismatch issues, I agree with you, this is probably easy to prevent by stabilizing your initial state.

The other issue is still relevant and unrelated to SSR, but rather to the fact that it is dependent on the component lifecycle. The current implementation allows for using machines in a global context. This global context might not be tied to a component. As long as a globally used machine is not used within a component context state transitions will be lost, which I'd consider a bug. This bug could be prevented if the machine is started once it is injected, but from your example that would lead to the error being thrown on the server side. I guess I don't understand the issue around the error being thrown on the server side properly, to make some sort of decision on this issue.

niklas-wortmann commented 5 months ago

@Andarist how do you want to proceed? I can delete/fix the test or adjust the implementation?

matheo commented 2 weeks ago

@niklas-wortmann thanks for this! I'm just getting to know xstate and I needed a ngrx-signals'like adaptation <3

matheo commented 1 week ago

@Andarist playing with this I wonder one thing: If I create a machine and an actor separately, they can communicate each other via systemId? or they need to be in the same parent/child hierarchy? I ask this because in the Angular context we create the instances "on demand", and we are used to have independent services which would be actors in this case.

So I'm trying to create a Machine that consumes different actors via systemId to access the Angular providers/instances.

It works defining the actors inside the machine and using spawnChild, but it doesn't work just by creating the actor in another section of the app with createActor(... { ..., systemId }), so the actor needs to be created in the machine scope to be referenced via systemId? Can you give me some hints if it's possible to create them independently please? TIA

SerkanSipahi commented 2 days ago

any updates?