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

[simplebar-vue] fix: move vue-demi to dependencies #629

Conversation

renatodeleao
Copy link
Contributor

@renatodeleao renatodeleao commented Nov 28, 2022

First I would like to apologise for breaking the 1.7.0 build :/
I've trusted unit testing and forgot to install it externally to ensure it works in real life, or should have asked you to release it as beta.

import { isVue3 } from 'vue-demi';
export const lifecycleEventNames = {
beforeUnmount: isVue3 ? 'beforeUnmount' : 'beforeDestroy',
unmount: isVue3 ? 'unmount' : 'destroy'
};

  1. vue-demi is not only used for testing but also to dynamically create the lifecyle names map for cross-compatiblity with vue 2 and 3
  2. updated vue-demi to latest version, I was going crazy to make the tests work and thought that vue-demi had something to do with it but I has not.

- I'm very sry for breaking 1.7.0 build, but vue-demi is not only used for testing but also to dynamically create the lifecyle names map for cross-compatiblity with vue 2 and 3
- updated vue-demi to latest version
@Grsmto
Copy link
Owner

Grsmto commented Nov 28, 2022

I see. That's my bad I tested that badly I think. I'm merging this now and giving it a test (I'm adding a new example in the /examples folder specifically for vue 3). If that works I'll release a patch.

@Grsmto Grsmto merged commit 457d64b into Grsmto:master Nov 28, 2022
@Grsmto
Copy link
Owner

Grsmto commented Nov 28, 2022

@renatodeleao I just published a patch, if you can give it a try!

@renatodeleao
Copy link
Contributor Author

renatodeleao commented Nov 28, 2022

@Grsmto tried, from my tests it works on vue3 but not on vue2 even prefixing the build script with npm run use-vue:2.7 does not change the outcome (not sure why).

It seems that I underestimated the task: my past experience with cross-compatible components I used solely render functions and didn't involve compiling SFC (.vue) and that's apparently where the problem is: the compiled output of SFC is a render function tailored for vue3 that does not work in vue2. There's scattered information around the web on how to do it right and these issues on vue-demi have listed the most workarounds but none is considered a de facto solution.

Since I've promised vue2 and vue3 support and now have to deliver, what I can do in the short-term to mitigate the failed release is basically re-writing the <template> as a plain render function with some conditionals for handling the differences between vue 2 and 3 signatures. I already managed to make it work for vue2.7 and vue3 renatodeleao@33146ca , but I'm not sure If I'm going to make it for vue 2.6 which if becomes true tomorrow, means a breaking change version.

Once again, sry for the trouble, I had the best intentions when I suggested this feature didn't expect so many issues down the road!

@Grsmto
Copy link
Owner

Grsmto commented Nov 28, 2022

@renatodeleao indeed I have tested only the vue 3 version but the vue 2 is not working. I will revert to the previous vue 2 support for now to avoid too many side effects from people upgrading to latest version.

@renatodeleao
Copy link
Contributor Author

@Grsmto You did well in reverting, thanks and sry!
The good news is that I have a — hopefully 🤞 — working version that is fully cross-compatible. Will push it sometime today!

@Grsmto
Copy link
Owner

Grsmto commented Nov 29, 2022

Great stuff. Let's not do the same mistake and do a pre-release to test it this time! 😅

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

Successfully merging this pull request may close these issues.

2 participants