-
-
Notifications
You must be signed in to change notification settings - Fork 274
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
fix: Run correctly when used alongside @vue/compat
#676
Conversation
|
Someone is attempting to deploy a commit to a Personal Account owned by @bcakmakoglu on Vercel. @bcakmakoglu first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@vue/compat
@@ -1,5 +1,5 @@ | |||
<script lang="ts" setup> | |||
import type { PanelProps } from '../../types' | |||
import type { PanelProps } from '../../types/panel' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without this change, the build failed with the following error after I added the non-setup <script>
tag:
I referenced Handle.vue
which also has a non-setup <script>
block and this appeared to be the only difference between them; indeed, it fixed the build. I'm not familiar with <script setup>
, though - so I'm not sure if this is the correct fix or not. It was also the only component that had this problem.
Thanks for the PR, I'll check it out 👍 |
appreciate it @bcakmakoglu! I don't want to rush you but it would be super helpful if I could get this through quickly (want to avoid getting blocked on deploying with this library after finishing my prototype) |
I’ll include the PR in tomorrows release |
yes, tomorrow would be perfect! Thank you! to answer your second question - it's definitely necessary for the functional components, but I'm not sure if it's strictly necessary for the non-functional ones. There are a lot of different flags that sometimes subtly change functionality (like how I also wanted to say, I love how you have your repo set up! Once I got docker installed and running development was a breeze 🏃♂️ - will probably end up spending some time looking at how things are set up here and take some learnings back to my own project |
@snoozbuster May I ask if you could update your commit message to adhere to conventional commits? Other than that, this looks good to me 👍 I can add a changeset afterwards to patch the package versions |
@bcakmakoglu yep, done! thanks again! |
@vue/compat
@vue/compat
🚀 What's changed?
@vue/compat
🐛 Fixes
I didn't file a bug report because I'm lazy. However, when running VueFlow in a Vue 3 project that has
@vue/compat
installed, the package crashes when rendering edges. This is because of the change in the way functional components work in Vue 3 (simple functions are now functional components, but unless the ASYNC_COMPONENT flag in@vue/compat
is disabled, it will attempt to process them as legacy-style async components which just... totally does not work).I did test this in my project by running
pnpm build
and then usingnpm link
to link@vue-flow/core
into my project. Before this change, big crash. After this change, no crash and I'm able to render a basic chart without errors.🪴 To-Dos
No todos to my knowledge 🎉 I believe I got all the components across both TS and Vue files across all packages but please let me know if I missed any. I'm no stranger to these types of PRs - I've made similar ones for @vue/test-utils and vue-router some time back (also, VueDraggable - but that package appears to not be maintained anymore). This package looks great and I'm excited to try it out in my project once this gets merged!