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

feat(#238): isomorphic vue2 and vue3 #563

Merged

Conversation

renatodeleao
Copy link
Collaborator

@renatodeleao renatodeleao commented Jul 6, 2022

@ndelvalle long time no see (read) 👋

Happen to be migrating my client's codebase to vue3, had some finally some time to dedicate to OSS :)
If you're around and have time to take a look at this let me know!
Hope you're doing alright, cheers!

Implementation Details

  1. Had a try using vue-demi as an attempt to make this library universal. Is more handy for the universal testing part than to the actual code, since we don't use any composition-api features yet.
  2. Anyways, I've added some automated tests asserting that it works on both versions: this was the actual bulk of work. Had to install vue 2 and 3 (under an npm alias vue3) and corresponding @vue/test-utils packages.
  3. The migration itself was just mapping bind, update, unbind methods to their vue3 counterparts beforeMount, updated, unmounted and was done in a minute.
  4. Ive also added vue requirements as peerDependencies, in conjunction with the aliased version mentioned above, makes npm install log a lot of warnings, and you'll have to pass --force flag to proceed. Once again i'm trying to copy what vue-demi actually did.

Closes #238

@renatodeleao renatodeleao force-pushed the feat-238/isomorphic-vue2-and-vue3 branch from 4cdbca6 to 9ae8469 Compare July 6, 2022 18:41
@renatodeleao renatodeleao changed the title Feat 238/isomorphic vue2 and vue3 feat(238): isomorphic vue2 and vue3 Jul 6, 2022
@renatodeleao renatodeleao changed the title feat(238): isomorphic vue2 and vue3 feat(#238): isomorphic vue2 and vue3 Jul 6, 2022
@renatodeleao renatodeleao force-pushed the feat-238/isomorphic-vue2-and-vue3 branch from 9ae8469 to 40f35c4 Compare July 6, 2022 19:08
@renatodeleao renatodeleao self-assigned this Jul 6, 2022
@renatodeleao renatodeleao marked this pull request as ready for review July 7, 2022 07:24
},
"peerDependencies": {
"@vue/composition-api": "^1.0.0",
"vue": "^2.6.0 || >=3.2.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "vue": "^2.0.0 || >=3.0.0"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory yes, but vue-demi only supports ^2.6.0 and since we depend on it, I opted per matching that limit.
On the 3.2.0 I don't actually remember why I did it 🤔 , maybe I had some errors while running the tests on prior v3 version, but will re-investigate.

@ndelvalle
Copy link
Owner

Hi @renatodeleao thank you for this awesome contribution ❤️, I will also add some examples to the examples folder and then release a new version 👍🏻

@renatodeleao
Copy link
Collaborator Author

renatodeleao commented Jul 11, 2022

@ndelvalle hold-on this for a bit longer. Installed on my real-world project (via git branch install) and some weird unrelated stuff started breaking, and i think it's related with vue-demi when paired with [email protected] + @vue/composition-api — it seems that the project starts bundling two version os @vue/composition-api

EDIT: related vueuse/vue-demi#151

@renatodeleao
Copy link
Collaborator Author

renatodeleao commented Jul 12, 2022

@ndelvalle update on ☝️

I don't think we'll have a "solution" soon. Depending on how consumer's vue project is bundled and which version they are using it might be necessary to apply extra configs while vueuse/vue-demi#151 and similar are sorted. So I guess we'll have to add a note to the readme, something along these lines:

### If you're using [email protected] + @vue/composition-api
If you start getting issues and suspect they came from duplicated `@vue/composition-api` calls like, the cause might be that `vue-demi` installed a second instance of `@vue/composition-api` plugin on your vue instance.

1. your `console.error` logs trace back to two different composition-api exports `.mjs` and `.common.js` as an example.
1. _the setup() binding property "x" is already declared_
1. etc

In this scenario you might need to tweak your bundler config to ensure that all your dependencies point to the same `@vue/composition-api` package:

#### Vite

// vite.config.js
// https://github.com/vueuse/vue-demi/issues/151#issuecomment-1088785153
resolve: {
  alias: [
    { find: '@vue/composition-api/dist/vue-composition-api.mjs', replacement: '@vue/composition-api' },
  ],
}
 
#### webpack

// webpack.config.js
resolve: {
  alias: {
     '@vue/composition-api$': 'path/to/node_modules/@vue/composition-api/dist/vue-composition-api.esm.js' // the one you want
  }
}

#### yarn resolutions

// package.json
// https://github.com/yarnpkg/rfcs/blob/master/implemented/0000-selective-versions-resolutions.md
{
  "resolutions": {
     "**/@vue/composition-api": "1.x" // or "file: ./path to the node_modules" version you want (depends on yarn version)
  }
}

Alternative

I guess the alternative is to drop this PR implementation relying on vue-demi and rolling a new one, probably branch based where main would be for vue2 and next would be for vue3. In my opinion, i don't think we have maintenance capacity for it.


Let me know what you think, when you have the time

@ndelvalle
Copy link
Owner

@ndelvalle update on ☝️

I don't think we'll have a "solution" soon. Depending on how consumer's vue project is bundled and which version they are using it might be necessary to apply extra configs while vueuse/vue-demi#151 and similar are sorted. So I guess we'll have to add a note to the readme, something along these lines:

### If you're using [email protected] + @vue/composition-api
If you start getting issues and suspect they came from duplicated `@vue/composition-api` calls like, the cause might be that `vue-demi` installed a second instance of `@vue/composition-api` plugin on your vue instance.

1. your `console.error` logs trace back to two different composition-api exports `.mjs` and `.common.js` as an example.
1. _the setup() binding property "x" is already declared_
1. etc

In this scenario you might need to tweak your bundler config to ensure that all your dependencies point to the same `@vue/composition-api` package:

#### Vite

// vite.config.js
// https://github.com/vueuse/vue-demi/issues/151#issuecomment-1088785153
resolve: {
  alias: [
    { find: '@vue/composition-api/dist/vue-composition-api.mjs', replacement: '@vue/composition-api' },
  ],
}
 
#### webpack

// webpack.config.js
resolve: {
  alias: {
     '@vue/composition-api$': 'path/to/node_modules/@vue/composition-api/dist/vue-composition-api.esm.js' // the one you want
  }
}

#### yarn resolutions

// package.json
// https://github.com/yarnpkg/rfcs/blob/master/implemented/0000-selective-versions-resolutions.md
{
  "resolutions": {
     "**/@vue/composition-api": "1.x" // or "file: ./path to the node_modules" version you want (depends on yarn version)
  }
}

Alternative

I guess the alternative is to drop this PR implementation relying on vue-demi and rolling a new one, probably branch based where main would be for vue2 and next would be for vue3. In my opinion, i don't think we have maintenance capacity for it.

Let me know what you think, when you have the time

Thanks for the research @renatodeleao. Yes, this is a tricky situation. I wonder how many people are using vue3 vs vue2.
What do you think about this approach:
Release a new version exposing a vue3 compatible API but also export the JavaScript functions to be able to also make it work with vue2.
We can add examples on how to use this new version with vue2 using the core exported functionalities of the lib, it will require a bit more work for vue2 but for vue3 will work out of the box.

@renatodeleao
Copy link
Collaborator Author

renatodeleao commented Jul 15, 2022

@ndelvalle I'll give one last shot trying to force dependencies as external as per latest issue comment vueuse/vue-demi#151. I will try to install this on a clean state vue2.6 + composition-api + vue-2.7 and vue3 instead projects and see what happens.

Our usecase is very simple, so yes we can also try that approach. The only downsides i'm seeing is that, by tweaking the code that way, it would be a breaking change for vue2 users and there's still a lot of people stuck with it.. so i don't know... I suspect that the majority of our users are still on vue2, but this is just a guess, because the edgy ones on vue3 maybe moved to another package in the meantime. Also, what would we do if we find a bug and need to patch the library? The vue2 users would have to go through a breaking change just to get that fix since the new version (4.x.y) no longer works for them out-of-the-box. 🤔

There's also another option that is dropping vue-demi:

  1. we just use the isVue3 boolean and we can code it ourselves.
  2. we don't use any @vue/composition-api feature (yet), so the benefits that vue-demi of mapping exports to the correct packages are no useful to us.

The only reason to actually use vue-demi is just to keep our our code more future proof: if we want to dive into these new composition-api features in the future, like providing a useClickOutside composable to use inside setup() as an example, we can just do it™

I'll let you know when I have news, enjoy the weekend! 👋

also included vue 2 and 3 as devDependencies for testing
This is "almost" an "integration" test, to ensure the code works on actual vue environment. Add plugin instalation tests.

SIDE-EFFECTS:
- shoutout to the Dobromir for the code vueuse/vue-demi#38 (comment)
- forward correct test utils exports
- add ifVue2 and ifVue3 test utils, when testing breaking changes — code that is not inter-compatible
- add universalDestroy, because lifecycle hook names changed and this prevents the version check on test code
these are now asserted on both vue2 and vue3 context via vue-
@renatodeleao renatodeleao force-pushed the feat-238/isomorphic-vue2-and-vue3 branch from 40f35c4 to 4ddf640 Compare July 19, 2022 14:21
@renatodeleao
Copy link
Collaborator Author

renatodeleao commented Jul 19, 2022

@ndelvalle update: i've decided to remove vue-demi 59c518e - ⚠️ rebase alert.

The benefits we we're getting were not outweighing the cons: one extra dependency + mandatory @vue/composition-api install on consumers projects when we don't actually use any new vue3 (or backported vue2.7) composition features.

  1. I've refactored with a simpl isVue3 regex test matching the version property that vue export includes.
  2. I kept isomorphic testing because it's nice way to assert things are working
  3. But as a check, i've tested on clean vue2 and vue3 projects and works perfectly.

If we ever get into using new vue composition functions, to avoid any mess we should just simply drop [email protected] support (or create a separate legacy branch) and support only vue2.7.x which makes sense as it this later will reach EOL by next year.

Let me know if you want me to squash commits before merge to remove this vue-demi experiment from git history.
Cheers!

@renatodeleao renatodeleao force-pushed the feat-238/isomorphic-vue2-and-vue3 branch 2 times, most recently from 411ccce to a8b3def Compare July 19, 2022 16:11
using vue-demi requires that vue2.6 projects install "@vue/composition-api", always! Even though I believe is the norm in 2.6.x projects but still kind of bummer to get an install error. vue-demi lists @vue/composition-api as optional peer dependency because it would otherwise install it for vue2.7 and vue3.x projects. We could list @vue/composition-api as dependency as well to overcome the install issue, but we're not using anything on this package so why complicate things?

1. I've removed vue-demi altogether, replaced the only thing we've use isVue3 with simply version regexp
1. the swap-vue.js from vuelidate allow us to keep the isomorphic testing to ensure it works everything

Related issues:
vueuse/vue-demi#67
vueuse/vue-demi#7
@renatodeleao renatodeleao force-pushed the feat-238/isomorphic-vue2-and-vue3 branch from a8b3def to 59c518e Compare July 19, 2022 16:13
@ndelvalle ndelvalle merged commit 22339a8 into ndelvalle:master Jul 27, 2022
@ndelvalle
Copy link
Owner

@renatodeleao thanks for the work on researching and implementing this PR. I'll create a new vue3 project on the /examples folder to have an easy to test small project.

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

Successfully merging this pull request may close these issues.

Vue 3?
2 participants