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

Bug: @xstate/vue can't be used reliably to create a shared machine across components #4754

Open
IonianPlayboy opened this issue Feb 21, 2024 · 5 comments
Labels

Comments

@IonianPlayboy
Copy link

IonianPlayboy commented Feb 21, 2024

XState version

XState version 5

Description

I need to use the same instance of a machine across different components, because the machine serves as the source of truth for the state of a core business logic in my app.
The most idiomatic way of doing this in Vue (to my knowledge) would be to create a custom hook wrapping the base useMachine invocation with createSharedComposable, and then to use this custom hook everywhere I need to have this shared instance.

This is sadly not working currently in a couple of scenarios, because the machine is stopped prematurely and won't restart afterwards. It's really a huge pain point for me, since it means that currently I can't use @xstate/vue as soon as my needs start to grow too big for its scope.

Expected result

I have created a reproduction to highlight the issue, with the expected behavior showing on the right panel (named FixedPageViews) :

  • Open the repro, landing on the "Home page".
image

Since our machine only tracks page views for the child routes of the PagesToDiscover page, currently FixedPageViews should only display its name, with no counter of page views below.

  • Click on one of the link at the bottom of the page that is not "home".
image

FixedPageViews should now be displaying that the page you just visited has been viewed once.

  • Click on other links that are not "home" any number of times.
image

FixedPageViewsshould be displaying the correct count of views for each page you clicked.

  • Click on the "home" link to go back to the home page
image

FixedPageViewsshould correctly have reset to its initial state, since we unmounted the PagesToDiscover component before going back to Home.

Since it is back to its initial state, you should be able to restart this flow from the start and observe the same results.

Actual result

The current behavior is showed in the repro on the left panel (named PageViews) :

  • Open the repro, landing on the "Home page".
image

Since our machine only tracks page views for the child routes of the PagesToDiscover page, currently PageViews should only display its name, with no counter of page views below. This is the correct behavior.

  • Click on one of the link at the bottom of the page that is not "home".
image

PageViews should now be displaying that the page you just visited has been viewed once. This is the correct behavior.

  • Click on other links that are not "home" any number of times.
image

PageViews does not update at all from this point, since the machine has already been stopped. This is not what is expected. If you open the console, you can see the warning and errors sent by XState about it :
image

  • Click on the "home" link to go back to the home page
image

PageViewsshould correctly have reset to its initial state, since we unmounted the PagesToDiscover component before going back to Home. This is the correct behavior.

Since it is back to its initial state, you should be able to restart this flow from the start and observe the same results.

Reproduction

https://stackblitz.com/edit/github-mwzbpb?file=src%2FpageViewsMachine.ts

Additional context

I have investigated to understand where the issue is coming from, and the culprit is the current implementation of useActorRef in @xstate/vue:

onMounted(() => {
if (observerOrListener) {
sub = actorRef.subscribe(toObserver(observerOrListener));
}
actorRef.start();
});
onBeforeUnmount(() => {
actorRef.stop();
sub?.unsubscribe();
});

The use of onBeforeUnmount here create a tight coupling between the machine instance and the lifecycle of the component that use it. In the reproduction, when we switch from a child route to another one, the first child is unmounted before we can go to the next page, which will stop this machine instance indefinitely.

The more straightforward solution to this problem to me is the one I used in the reproduction for the fixedPageViews: replacing the onMounted/onBeforeUnmount by effectScope/onScopeDispose, which is intended to replace the explicit component lifecycle hooks when used in a composable.

More details on these APIs are available in the related implementation PR and RFC:

The main problem with this solution is that it would technically introduce a breaking change, since these APIs are only available for Vue ^3.2.0 and currently XState is supporting the ^3.0.0 range.

I guess that could be an opportunity to upgrade the installed Vue version in the repo, since I believe I have seen in the source code that the global JSX declaration from Vue was causing some issues, and it has been removed in 3.4.

If this is not a big enough concern to prevent fixing this bug, I would be interested in opening a PR to propose my solution if that's okay.

@Andarist
Copy link
Member

The idiomatic way to achieve this would be to hoist your machine to some common parent and then pass down the created actorRef using the provide/inject API ( https://vuejs.org/guide/components/provide-inject ). useMachine/useActor starts separate instances so that's not what you are looking for.

I'll use a React code snippet here, since that's what I'm familiar with:

const counterMachine = setup({}).createMachine({ /* ... */ })

const First = () => {
  useActor(counterMachine)
  return null
}


const Second = () => {
  useActor(counterMachine)
  return null
}

const App = () => {
  return (
    <>
      <First />
      <Second />
    </>
  )
}

This application doesn't run a single machine but rather two machines - each one is scoped to its own component.

@IonianPlayboy
Copy link
Author

IonianPlayboy commented Feb 21, 2024

The idiomatic way to achieve this would be to hoist your machine to some common parent and then pass down the created actorRef using the provide/inject API ( https://vuejs.org/guide/components/provide-inject ).

Indeed, this would be a suitable workaround and do exactly what I need without any changes needed in @xstate/vue. I overlooked this alternative because I'm much more used to use hooks instead when possible, and this is less ideal for me since it would force me to use two different ways of interacting with machine depending on if they should be shared or not, but it would work just fine.

useMachine/useActor starts separate instances so that's not what you are looking for.
[...]
This application doesn't run a single machine but rather two machines - each one is scoped to its own component.

I understand that and I never expected that simply using useMachine/useActor out of the box would create a shared machine/actor across components. I guess I did not make it clear in the issue that I was wrapping the useMachine hook inside a function that would make it shareable, so to better reflect my intent your React snippet would look more like this :

const counterMachine = setup({}).createMachine({ /* ... */ })

const useSharedCounterActor = () =>
  createSharedHook(() => useActor(counterMachine));

const First = () => {
  useSharedCounterActor();
  return null;
};

const Second = () => {
  useSharedCounterActor();
  return null;
};

const App = () => {
  return (
    <>
      <First />
      <Second />
    </>
  );
};

Now First and Second would share access to the same instance of the counter machine, instead of having each their own separate instance.

In Vue, this is possible thanks to effect scopes. To sum it up, every reactive effects (ref, computed, watch, etc...) are bound to a scope, and are disposed/collected when the scope is terminated. This is how Vue keep track of everything and knows when it should stop and garbage collects what's not needed anymore.

By default, each component define its own scope, so reactive effects are disposed when their parent component is unmounted. However, the effectScope API allows to define a custom scope that is not bounded to a particular component, and it can be used to create a shareable hook that will keep track of the places where its reactive effects are needed (i.e its suscriber), and can stop itself when all the related components have been unmounted.

There is an example of how it can be implemented in the RFC that introduced this API if you want to learn more about it, but what's important here is that creating shareable hooks is possible in Vue 3.2.x and beyond. There is even an utility function provided by a popular Vue library that allow to do exactly that, without having to code the logic yourself.

That's why I expected at first that by wrapping useMachine/useActor inside a createSharedComposable utility, I would be able to reuse the same machine across components. However, because useActorRef calls directly onBeforeUnmount, its reactive effects are stopped independently from its parent scope, which breaks the shared instance when a component unmount, for example when changing routes. On the repro, the "fixed" version of the machine hook/composable uses a patched useActorRef that implements the effectScope API , which is why it is working as intended vs the out-of-the-box version.

Personally, I still consider the current behavior of @xstate/vue to be a bug, or at least unexpected behavior, but as you pointed out a workaround is available. I would understand if you considered that it makes fixing this issue a noop or at least not your priority, especially since it would require to upgrade the vue version to at least 3.2.x, which is a breaking change.

I hope that I was able to clear up any misunderstanding and that the explanations I gave were clear enough, if that's not the case feel free to ask me any precisions you need.

Thank you for your time and for maintaining this project. 😊

@FrankySnow
Copy link

Unless I'm missing something, provide/inject is easy until you want to add type safety to it, which causes some indirections that add boilerplate and in the end force you to type-cast something (either the injection key and/or the provided value) :

// keys.ts
import type { InjectionKey } from 'vue'
import { Actor } from 'xstate'
import { authMachine } from './actors/authMachine'

export const authKey = Symbol('auth') as InjectionKey<Actor<typeof authMachine>>
// Parent component
import { authMachine } from 'src/actors/authMachine'
import { authKey } from './keys'
import { provide } from 'vue'

const { actorRef } = useActor(authMachine.provide(/* ... */)
provide(authKey, actorRef)
// Child component
import { authKey } from 'src/keys'
import { inject } from 'vue'

const throwError = () => { throw new Error() }
// typeof inject(authKey) is Actor<typeof authMachine> | undefined
// which is not usable, so I have to narrow it like this :
const authActor = inject(authKey) || throwError()

Please correct me if I'm doing something wrong here, but IMHO an API using shared composables would be better suited and more appealing to existing Vue developers, as proposed by @IonianPlayboy.

@davidkpiano
Copy link
Member

Please correct me if I'm doing something wrong here, but IMHO an API using shared composables would be better suited and more appealing to existing Vue developers, as proposed by @IonianPlayboy.

I would gladly welcome a contribution for this.

@Andarist
Copy link
Member

Unless I'm missing something, provide/inject is easy until you want to add type safety to it, which causes some indirections that add boilerplate and in the end force you to type-cast something (either the injection key and/or the provided value)

To be fair, that's kinda on Vue's APIs. I agree that this is somewhat awkward and I prefer how the same thing is done in React.

In @xstate/react we have createActorContext that binds a couple of pieces together (type-wise). A similar approach could be used here - that would hide inject/provide dance behind the scenes.

Please correct me if I'm doing something wrong here, but IMHO an API using shared composables would be better suited and more appealing to existing Vue developers, as proposed by @IonianPlayboy.

That's somewhat similar to createActorContext idea. I'm not sure how the @IonianPlayboy's idea would be implemented though. It refers to createSharedHook but it doesn't go into its details and - from what I can tell - this is not something that Vue provides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants