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

@xstate/vue not working with vue beta #1252

Closed
mkhennoussi opened this issue Jun 19, 2020 · 29 comments · Fixed by #1629
Closed

@xstate/vue not working with vue beta #1252

mkhennoussi opened this issue Jun 19, 2020 · 29 comments · Fixed by #1629

Comments

@mkhennoussi
Copy link

Description
@xstate/vue is not working with vue beta. The module still use @vue/composition-api as a dependency. Should we drop it as vue has enter beta ?

Actual Result
when I use the example in the readme, I get an error like

Uncaught SyntaxError: The requested module '/@modules/@xstate/vue/lib/index.js' does not provide an export named 'useMachine'

Reproduction
Create a new project with vue-cli or vite
To use vue 3 (with composition api) with vue-cli you need to do : vue add vue-next
Then just write the example in the readme file, you should get the error.

I am pretty new to xstate and all the concepts behind it, so I am not sure how to start.

Any ideas ? Thanks a lot !

@davidkpiano
Copy link
Member

I would accept a PR for this 🙏

@Andarist
Copy link
Member

We don't use @vue/composition-api as a dependency but rather as a peer dependency. I'm not sure but maybe this PR will resolve this issue? If not - please create a repro case so we could take a look at it.

@santicros
Copy link
Contributor

I wanted to try @xstate/vue with Vite and Vue 3 and can confirm it doesn't work. It still tries to import @vue/composition-api.

One of the errors I get is:

[vite]: Rollup failed to resolve import "@vue/composition-api" from "@vue/composition-api?commonjs-external".

Here I created a repo for the example with the steps below: Reproduction Repo

Reproduction:

1. Create project with Vite:

$ npm init vite-app <project-name>
$ cd <project-name>
$ npm install

2. Install xstate and @xstate/vue:

$ npm i xstate @xstate/vue

3. Create test machine from docs

<template>
  <button @click="send('TOGGLE')">
    {{
      state.value === 'inactive'
        ? 'Click to activate'
        : 'Active! Click to deactivate'
    }}
  </button>
</template>

<script>
import { useMachine } from '@xstate/vue';
import { Machine } from 'xstate';

const toggleMachine = Machine({
  id: 'toggle',
  initial: 'inactive',
  states: {
    inactive: {
      on: { TOGGLE: 'active' }
    },
    active: {
      on: { TOGGLE: 'inactive' }
    }
  }
});

export default {
  setup() {
    const { state, send } = useMachine(toggleMachine);
    return {
      state,
      send
    };
  }
};
</script>

4. Try to run it

$ npm run dev

Again, here is a repo from the steps above: Reproduction Repo

@davidkpiano
Copy link
Member

@AleksejDix Would you know what we would need to do here?

@chopfitzroy
Copy link

chopfitzroy commented Sep 11, 2020

Add this here, I actually get this error with using @xstate/react with vite as well.

Uncaught SyntaxError: The requested module '/@modules/@xstate/react.js' does not provide an export named 'useMachine'

@sarahdayan
Copy link
Contributor

sarahdayan commented Sep 12, 2020

There are two ways to use the Composition API:

The problem comes from the fact that this file tries to import Composition functions from @vue/composition-api. This is why @xstate/vue needs it as a peer dependency; yet in Vue 3 beta this isn't compatible.

If you want @xstate/vue to be available in Vue 2 with the @vue/composition-api plugin, you'll likely want to expose two entry points and set @vue/composition-api as an optionalDependency. If you don't, and only care about the Composition API in Vue 3, you can switch the peer dependency to Vue and change the import source. That would require a major release though.

Let us know what your vision is for this @davidkpiano. I'd be happy to PR.

@davidkpiano
Copy link
Member

@sarahdayan Thanks so much for this. My assumption is that Vue developers who want to use the composition API are probably already using Vue 3 beta, so Vue 2 developers can use the implementation in the docs. Does this seem accurate?

If that's right (and we want to avoid supporting multiple entry points), I would vote to do the second option (switch peer dep to Vue, major release).

What do you think?

@sarahdayan
Copy link
Contributor

sarahdayan commented Sep 12, 2020

@davidkpiano My thought is that the Composition API is ultimately meant to be used in Vue 3.

The Vue 2 plugin was a great way to discover the API and get familiar with it while Vue 3 was being developed, but there's a high chance that it ends up being deprecated and users are encouraged to upgrade. The plugin has limitations and a performance impact in Vue 2 because the Vue 2 API wasn't designed for it.

Of course, this is only my guess as an individual Vue developer. I'm not a Vue team member, nor do I speak for the entire community. I asked on the repo what the future of the plugin is to get more objective data. Additionally, as soon as Vue 3 comes out (which should be anytime now since it's in RC as we speak), Vue 2 will go in LTS for no longer than 18 months (and then another 18 months in maintenance mode). Finally, you can see a significant drop in npm downloads for @vue/composition-api over the last few weeks.

Capture d’écran 2020-09-12 à 17 05 28

I would personally recommend going with the second option as well. It's going to be much easier to maintain, and users willing to try the XState Composition functions within Vue 2 with the @vue/composition-api plugin can still install the current major version (which we can document).

UPDATE: As suggested by this reply, the plugin isn't likely to get deprecated. It will go to v1 and hopefully, the last minor release of Vue 2 will backport enough features to remove performance issues and limitations. Yet, there's a plugin that allows library maintainers to write Vue 2/3 universal APIs, that could be an interesting solution. Based on the detected local version of Vue, it either installs the @vue/composition-api plugin or not with a postinstall script, and routes the imports to the right package (vue or @vue/composition-api). The plugin is non-official and experimental though, so it could bring additional bugs. It may be a big risk for such a critical dependency. The way I see it, that solution could be considered if @xstate/vue users massively start asking for it once the new major is out.

@sarahdayan
Copy link
Contributor

What are your thoughts @davidkpiano?
Do you prefer going with a breaking new version and drop @vue/composition-api, or do you want to preserve compatibility via vue-demi?

@davidkpiano
Copy link
Member

Let's go the compatibility route if it doesn't add complexity or other issues. Vue-demi looks good!

@Andarist
Copy link
Member

Has vue-demi been confirmed to work with pnpm and Yarn 2? The postinstall script - while nice trick - seems like a can of worms to me 😅

@sarahdayan
Copy link
Contributor

sarahdayan commented Sep 24, 2020

@davidkpiano Using vue-demi will definitely add complexity, and may bring issues:

  • Testing is going to be non-trivial. The vue-demi package modifies its own source code upon install, based on the installed Vue version. This won't be testable with the existing testing setup. To make sure that @xstate/vue works both within a Vue 2 and Vue 3 project, we would need to emulate both environments and run tests within them.
  • The package is very young and experimental. Bugs may definitely arise, it could be a big risk for such a critical dependency.
  • As @Andarist said, the fact that vue-demi relies on postinstall may become an issue.

If you want to preserve compatibility for now, what do you think about adding new entry points?

// Rely on `vue`
import * as XState from '@xstate/vue';
import * as XStateFSM from '@xstate/fsm';

// Rely on `@vue/composition-api`
import * as XStateCmpPlugin from '@xstate/vue/composition-plugin';
import * as XStateFSMCmpPlugin from '@xstate/fsm/composition-plugin';

The existing code should be easy to share with dependency injection. It will also be much easier to test.
This would require a new major. Once Vue 2 enters maintenance mode (in 18 months), the composition-plugin entry point can be deprecated.

@davidkpiano
Copy link
Member

Hmm, thanks for the info. I thought it would be more seamless. Maybe it would be best to only target Vue 3 then?

For Vue 2, integrating XState can be done manually (as is explained the docs).

@sarahdayan
Copy link
Contributor

Yeah, so we'd go with vue@3.0.0-rc.1 (or above) as the peer dependency to retrieve Vue's composition functions. Since the package is still in v0, I guess it's okay to simply go up a minor (as defined by semver).

For Vue 2, the recommendation from the recipe is to use the Options API. Vue 2 users who are already using @xstate/vue's composition functions with @vue/composition-api will have to either stick to the current version of the package, move back to the Options API, or upgrade to Vue 3 (which we'll need to document here and here). If they didn't pin their dependencies, things may break for them, but according to semver rules, that's their responsibility. Are we good on this?

If yes, I'll PR soon 🙂

@davidkpiano
Copy link
Member

Are we good on this?

Yes! ✅

@sarahdayan
Copy link
Contributor

Finally found some time to work on this! I'm trying to get the project running but encountering issues after install @davidkpiano. The install is successful, but I'm getting a bunch of errors when running tests on master (many of them due to modules not found, even though Node modules were properly installed). I couldn't find any specific indications besides the contributing guide to set things up.

Anything I should be aware of @davidkpiano?

@davidkpiano
Copy link
Member

@sarahdayan Thanks so much! Do you have this in a repo somewhere?

@sarahdayan
Copy link
Contributor

sarahdayan commented Oct 26, 2020

@davidkpiano I tried this on my fork. At first I thought it was my changes, but I checked out master and ran tests after installing dependencies, and got a whole bunch of errors.

@Andarist
Copy link
Member

@sarahdayan the setup should be as easy as running yarn in the root. You can later develop (have the repo rebuilt automatically with jest running on the whole repo as well) by using yarn dev in the root. I believe those are mentioned in the contributing guide but if you find a problem with any of this on your machine then it might be best to catch me on Twitter (my DMs are open) for a more lively conversation.

@sarahdayan
Copy link
Contributor

@Andarist Yep that's what I did (run yarn at the root to install dependencies) then run yarn test to see tests succeed (which should be the case on master) and they failed. The contributing guide doesn't mention yarn dev though (are we talking about this one)?

I'll give it another look and will ping you on Twitter if I can't get it to work.

@Andarist
Copy link
Member

Andarist commented Oct 27, 2020

@sarahdayan sorry - I've subconsciously mapped yarn test in the contributing guide to yarn dev which I'm using when developing. I'll adjust the guide later to explain both options - for me yarn dev works great because it's a combined yarn build & yarn test (with just some extra lines on top of that), with just yarn test u might end up with your tests running against outdated dist files because sources are not automatically rebuilt to dist files on code changes.

The next branch has an easier setup - things just work, so that's a good thing but we can't backport that setup to master because it would be a breaking change for some packages 😢

@noor-tg
Copy link

noor-tg commented Nov 6, 2020

any progress about this issue ?

@pearofducks
Copy link
Contributor

pearofducks commented Nov 8, 2020

I started taking a look at this as well. @Andarist @sarahdayan thoughts on 👇 ?

  1. The renderless component feels out of place here to me. It's missing tests, it isn't clear what should/shouldn't be reactive, and has some implementation choices that aren't clear and aren't commented on.[0]

    • IMO this component should be removed from the official package and live on in userland.
  2. The use of both @vue/test-utils and @testing-library feels redundant, and requires one to know the API for both, instead of just one or the other. This isn't a big deal IMO, but it does feel like a lot of extra wiring going around for tests.

  3. It'd also be great to get some ESM packaging published with this change, since Vue 3 treats ESM as a first-class citizen.

[0] e.g.

  • why the beforeMount hook is used for starting the service
    • this might just be my ignorance here, but I could see the result of this component being used in a SSR context, and this hook is client-only
  • why there's 2-3 extra unused computed (or in one case not really necessary)

@Andarist
Copy link
Member

Andarist commented Nov 8, 2020

@pearofducks could u link to exact lines that you are asking about? It would ensure that we can be on the same page right from the start.

@pearofducks
Copy link
Contributor

pearofducks commented Nov 8, 2020

@Andarist - sure!

Related to the renderless component:

  • The computed here aren't used at all. The initialState computed above doesn't really need to be a computed, as it's never used reactively.

  • The only reactive aspect of the component is the the state machine itself changing from send - found here. I'm not really sure what benefit this component gives then vs. the useMachine hook. As one should be able to recreate this stateless component in just a few lines if they used that hook.

edit // I removed the beforeHook point - I was thinking of this behavior wrong!


Related to the test utilities:

  • Here both @vue/test-utils and @testing-library are used. Both are used for different tests, except to my knowledge they're interchangable with one another - so this test could be rewritten to just use one library instead of both.

Related to ESM:

  • The rollup config here only creates UMD, and there's no module declaration in the package.json. This makes future use with native browser ESM, as well as use with things like Snowpack rather difficult - and also makes it hard for Webpack or Rollup to tree-shake unused code from this module.

@pearofducks
Copy link
Contributor

@Nour-DEV @dennzimm -

I've published a stopgap package use-xstate, that has the same API as documented but will work with Vue 3.

This package should only be used for interim development while official support is added - but the APIs should match so switching packages once official support is added should be trivial.


As a side note: it's generally considered poor form to ping open-source projects asking for status updates. Please think of the fact that maintainers are generally volunteering their free time to make these things, and don't need additional pressure from people asking when something will be done.

Should you like faster progress, it's best to start finding out how you can help!

@Andarist
Copy link
Member

The computed here aren't used at all. The initialState computed above doesn't really need to be a computed, as it's never used reactively.

I would be fine with removing those computed properties if they are not used and cant be consumed by consumers right now.

The only reactive aspect of the component is the the state machine itself changing from send - found here. I'm not really sure what benefit this component gives then vs. the useMachine hook. As one should be able to recreate this stateless component in just a few lines if they used that hook.

I don't know much about Vue but this might have been implemented before useMachine was even a thing.

Here both @vue/test-utils and @testing-library are used. Both are used for different tests, except to my knowledge they're interchangable with one another - so this test could be rewritten to just use one library instead of both.

I'm fine with refactoring tests to only use one of them. Which one is preferred by the Vue community?

The rollup config here only creates UMD, and there's no module declaration in the package.json. This makes future use with native browser ESM, as well as use with things like Snowpack rather difficult - and also makes it hard for Webpack or Rollup to tree-shake unused code from this module.

This is already handled on the next branch but it will take some time before this branch gets published. Given that it's not high on our priority list - I could review a PR from an external contributor that would be implementing this though.

Should you like faster progress, it's best to start finding out how you can help!

❤️

@noor-tg
Copy link

noor-tg commented Nov 12, 2020

@Nour-DEV @dennzimm -

I've published a stopgap package use-xstate, that has the same API as documented but will work with Vue 3.

This package should only be used for interim development while official support is added - but the APIs should match so switching packages once official support is added should be trivial.

As a side note: it's generally considered poor form to ping open-source projects asking for status updates. Please think of the fact that maintainers are generally volunteering their free time to make these things, and don't need additional pressure from people asking when something will be done.

Should you like faster progress, it's best to start finding out how you can help!

@pearofducks thank you . the thing is I am very new to xstate and it's ecosystem. so I asked about the progress . but if there any thing I could help I would .

@sarahdayan
Copy link
Contributor

@sarahdayan sorry - I've subconsciously mapped yarn test in the contributing guide to yarn dev which I'm using when developing. I'll adjust the guide later to explain both options - for me yarn dev works great because it's a combined yarn build & yarn test (with just some extra lines on top of that), with just yarn test u might end up with your tests running against outdated dist files because sources are not automatically rebuilt to dist files on code changes.

Thank you! Working on the fix as we speak. Expect a PR soon.

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

Successfully merging a pull request may close this issue.

8 participants