feat: added vue 2 backwards compatibility for @vue-flow/background #1188

michealroberts · 2023-11-09T17:37:54Z

refactor: migrated vue 2 backwards compatibility for @vue-flow/background

Description

This MR refactors the @vue-flow/background package to be entirely SFC independent, allowing for backwards compatibility with Vue 2.7.* as well as continuing support for Vue 3.

Essentially, these changes should be able to be merged into master / main without any trouble at all for the existing package.

Testing 🔧

Vue 2 App Example (with just the background component rendered*):

Vue 3 Vite App Example:

changeset-bot · 2023-11-09T17:37:57Z

🦋 Changeset detected

Latest commit: 5a4ce17

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@vue-flow/background	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2023-11-09T17:37:57Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
vue-flow-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 10, 2023 0:22am
vue-flow-typedoc	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 10, 2023 0:22am

vercel · 2023-11-09T17:37:58Z

Someone is attempting to deploy a commit to a Personal Account owned by @bcakmakoglu on Vercel.

@bcakmakoglu first needs to authorize it.

.changeset/thick-moles-smash.md

bcakmakoglu · 2023-11-09T17:49:34Z

packages/background/src/Background.vue

As previously mentioned, please don't remove current component and instead add new ones.

Replacing components can still be done at a later point.

bcakmakoglu · 2023-11-09T17:52:03Z

packages/background/src/Background/index.ts

+const Background = /* #__PURE__ */ defineComponent({
+  name: 'Background',
+  compatConfig: { MODE: 2 },
+  props: {


Alas, I thought this might be the result of the syntax change, which imo, increases maintenance a lot.

Instead of defineProps<Props>() we now end up with this mess :(

I assume due to Vue 2 compatibility using the defineComponent<Props, Emits>({ ... }) will not work here, will it?

bcakmakoglu · 2023-11-09T17:53:06Z

packages/background/src/patterns.ts

Functional components are, I am pretty sure, Vue 2 compatible and seeing as these are stateless, I don't see why a refactor to stateful components (which in Vue 2 come with more overhead than in Vue 3, so there it would make much less a difference though)

bcakmakoglu · 2023-11-09T17:56:16Z

packages/background/src/Pattern/dot.ts

+    },
+  },
+  setup(props: DotPatternProps) {
+    const radius = computed(() => props.radius)


Might be I'm mistaken about available APIs in Vue 2 but:

computed comes with caching, which produces overhead and therefore has better usage when it comes to expensive computations, not when returning a prop or doing simple equality comparisons like prop.hello === 'world'
Can be replaced by toRef which doesnt produce the same overhead

Props should still be reactive, even without wrapping them in extra reactive variables.
Meaning, if you do return () => h('div', props.label) and update the label prop, it will still re-render appropriately without wrapping props.label in computed or toRef.

you Can open a separate repository for Vue2

bcakmakoglu · 2023-11-09T18:00:11Z

I'd tag this change as a feature instead of a refactor request as that's more in line with what's actually happening.

michealroberts · 2023-11-09T18:00:55Z

I'd tag this change as a feature instead of a refactor request as that's more in line with what's actually happening.

Cool. I think the changesets will need the appropriate minor bump too. Do you want me to take care of this?

bcakmakoglu · 2023-11-09T18:02:37Z

Changed the base branch to feat/vue2 so changes can be collected in there before release.

bcakmakoglu · 2023-11-09T18:12:58Z

I'm wondering how we could run the test suites for the Vue 2 components as well 🤔

michealroberts · 2023-11-09T18:52:07Z

I'm wondering how we could run the test suites for the Vue 2 components as well 🤔

What is your current test setup? Cypress?

michealroberts · 2023-11-09T18:52:26Z

I'd tag this change as a feature instead of a refactor request as that's more in line with what's actually happening.

Cool. I think the changesets will need the appropriate minor bump too. Do you want me to take care of this?

Taken care of the changesets.

bcakmakoglu · 2023-11-09T19:06:49Z

Yeah, tests are done with cypress component testing

bcakmakoglu · 2023-11-09T19:08:25Z

Could you possibly add a Vue 2 example App to the examples folder, so that this can be tried out?

michealroberts · 2023-11-09T19:09:33Z

Could you possibly add a Vue 2 example App to the examples folder, so that this can be tried out?

So I have added one locally, and it works well (not much use to you, granted) ... but we would need to migrate @vue-flow/core before we have full compatibility ...

*To get the Vue 2 app working locally, I have to strip the dependency in background on core, and hardest the values return by the useVlueFlow() composable.

bcakmakoglu · 2023-11-09T19:13:11Z

Why would you need to strip that? The app can use a different vue version without the whole repo using the same ^^

bcakmakoglu · 2023-11-09T19:13:37Z

You'd probably just need to remove the peerDepedency

bcakmakoglu · 2023-11-09T22:47:24Z

packages/background/src/Background/index.ts

+  name: 'Background',
+  compatConfig: { MODE: 2 },
+  setup(props: BackgroundProps, { slots }) {
+    const initialPatternColor = props.patternColor


Why aren't we allowing dynamic colors for patterns?

Users would expect changes to this prop to be reflected in the pattern color

bcakmakoglu · 2023-11-09T22:50:30Z

packages/background/src/Background/index.ts

+    const height = computed(() => {
+      return props.height || 100
+    })
+
+    const width = computed(() => {
+      return props.width || 100
+    })


Correct me if I'm wrong but if toRef is available in Vue 2 a simple toRef(() => props.height || 100) should be fine here as well?

bcakmakoglu · 2023-11-09T22:54:37Z

packages/background/package.json

+  },
+  "peerDependenciesMeta": {
+    "@vue/composition-api": {
+      "optional": true


Shouldn't this be mandatory?

bcakmakoglu · 2023-11-09T22:55:31Z

packages/background/package.json

  "peerDependencies": {
-    "@vue-flow/core": "^1.23.0",
-    "vue": "^3.3.0"
+    "@vue/composition-api": "^1.0.0-rc.1",


Why rc.1? Why not pick 1.0.0, the actual release?

bcakmakoglu · 2023-11-09T23:01:28Z

packages/background/src/Background.vue

If compat mode is set to 2 does the compiled out put of this SFC work in Vue 2 as well?
Imo this is a lot easier to maintain and read than the regular .ts file since the h syntax requires a lot of terniaries, making it look like a jsx mess.

The Background.ts file would also not emit any usable types atm (just doing props: SomeProps is not enough), giving the proposed implementation no auto-completion support

I tried it out and, alas, it does not work with the SFC, which sucks 😢

I guess the type issue could be circumvented by force casting the exposed components prop, emit and slot types.

Something like this, I believe? Could compare to what Volar outputs with the current roll strategy.

export const VueFlow = /* ... */ as { $props: FlowProps // ... }

bcakmakoglu · 2023-11-10T09:52:35Z

So after a bit of playing around, it turns out you can do defineComponent<Props>, which will roll up the proper types for the component and is usable in Vue 2.

const Background = /* #__PURE__ */ defineComponent<BackgroundProps>({
	// rest here...
})

That is a massive W tbh and should make this a lot easier.

Though defining default values for props is still a hassle 😄

Editor: My bad, it does roll types with no error but the types aren't recognized by Volar properly import("vue-demi").PropType<SomeType> sadly does not work, though import("vue").PropType<SomeType> does

Not the W I hoped for.

michealroberts · 2023-11-10T11:03:20Z

defineComponent<BackgroundProps>

Yeh this is a really strange one ... I'm wondering if this is a bug with Volar ... 🤔

bcakmakoglu · 2023-11-10T11:23:48Z

No idea :/

feat: migrated vue 2 backwards compatibility for @vue-flow/background

bcakmakoglu · 2023-11-10T12:10:45Z

Btw, appending + force pushing every commit you make messes up the comments on existing pieces of code ^^

You can just Squash the commits at the end instead of just forcing each time to preserve 1 commit

bcakmakoglu · 2023-11-10T12:12:07Z

packages/background/package.json

  "peerDependencies": {
-    "@vue-flow/core": "^1.23.0",
-    "vue": "^3.3.0"
+    "@vue/composition-api": "^1.7.2",


Wouldn't this unnecessarily force composition api to be installed for Vue 3 users who don't even need this package?

Still think this should be optional: false instead

michealroberts · 2023-11-10T12:13:25Z

@bcakmakoglu Yeh fair, sorry. Will squash at the end.

So some of my testing has found some quirky things to make this Vue 2 & Vue 3 compatible:

toRef doesn't work as expected with Vue 3, we have to use computed for Vue 2 to work reactively (I want to see if props are "reactive" inside render functions for both, will investigate this).
props need to be explicitly stated otherwise, Vue 2 does not pass them though correctly.
there is still a typing issue in the imported package, I am going to attempt to understand this today and reach out to the Volar team.

bcakmakoglu · 2023-11-10T12:13:57Z

packages/core/src/container/Pane/Pane.vue

-import UserSelection from '../../components/UserSelection/UserSelection.vue'
-import NodesSelection from '../../components/NodesSelection/NodesSelection.vue'


There's no index file to import from so this would atm cause an error

fix: remove accidental push

bcakmakoglu · 2023-11-10T12:29:33Z

toRef doesn't work as expected with Vue 3, we have to use computed for Vue 2 to work reactively (I want to see if props are "reactive" inside render functions for both, will investigate this).

Bummer but should be fine for now

props need to be explicitly stated otherwise, Vue 2 does not pass them though correctly.

Yeah, seems so - though for maintenance it might be better to define the props object in another file? iirc you can import object prop definitions

there is still a typing issue in the imported package, I am going to attempt to understand this today and reach out to the Volar team.

"a typing" issue or is it that the types aren't properly hinted in general?

michealroberts · 2023-11-10T12:34:59Z

Bummer but should be fine for now

I'll continue to dig into why this is, and understand the cause in the API disparity.

Yeah, seems so - though for maintenance it might be better to define the props object in another file? iirc you can import object prop definitions

Yeh I think we can do something like:

export default {
  props: {
    title: String,
    likes: Number
  }
}

"a typing" issue or is it that the types aren't properly hinted in general?

Yeh the hints is non-existing, and in the template usage where most of the mistakes will inevitably happen. I've reached out to the core team for some advice. I'm just not 100% sure ...

bcakmakoglu · 2023-11-10T12:57:15Z

I'll continue to dig into why this is, and understand the cause in the API disparity.

Well, toRef from @vueuse/core would probably work - I think the whole concept of MaybeRefOrGetter doesn't exist, even in Vue 2.7, making toRef(() => {} unusable anyway if I'm not mistaken.

toValue doesn't exist in Vue 2 either (which accepts a MaybeRefOrGetter type), which is another bummer.
This one was added pretty recently to the Vue 3 core API so no surprise here. Though this one is imported from @vueuse/core as I was using it even before it was part of the core API and atm there's an amigious export issue in the examples when I switch the import to vue (still investigating the reason for this problem)

Yeh the hints is non-existing, and in the template usage where most of the mistakes will inevitably happen. I've reached out to the core team for some advice. I'm just not 100% sure ...

Alright, let's wait and see :)

michealroberts · 2023-11-17T15:17:34Z

@Braks Closing this one as I will refactor on top of the core migrated PR.

michealroberts requested a review from bcakmakoglu as a code owner November 9, 2023 17:37

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 9, 2023 17:39 Inactive

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from b634310 to c0f5e57 Compare November 9, 2023 17:41

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 9, 2023 17:42 Inactive

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from c0f5e57 to e2850ce Compare November 9, 2023 17:51

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 9, 2023 17:51 Inactive

bcakmakoglu requested changes Nov 9, 2023

View reviewed changes

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from e2850ce to 7897750 Compare November 9, 2023 18:00

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 9, 2023 18:00 Inactive

bcakmakoglu changed the base branch from master to feat/vue2 November 9, 2023 18:01

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from 7897750 to 66f9a88 Compare November 9, 2023 18:51

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 9, 2023 18:52 Inactive

michealroberts changed the title ~~refactor: added vue 2 backwards compatibility for @vue-flow/background~~ feat: added vue 2 backwards compatibility for @vue-flow/background Nov 9, 2023

bcakmakoglu requested changes Nov 9, 2023

View reviewed changes

bcakmakoglu assigned michealroberts Nov 9, 2023

bcakmakoglu added the enhancement New feature or request label Nov 9, 2023

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from 66f9a88 to 474b5ba Compare November 10, 2023 09:46

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 09:47 Inactive

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from 474b5ba to c03a27e Compare November 10, 2023 09:49

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 09:50 Inactive

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from c03a27e to e7b4e09 Compare November 10, 2023 11:15

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 11:16 Inactive

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from e7b4e09 to e2ab4d4 Compare November 10, 2023 11:22

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 11:22 Inactive

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from e2ab4d4 to 7f756d7 Compare November 10, 2023 11:25

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 11:25 Inactive

feat: migrated vue 2 backwards compatibility for @vue-flow/background

95d49a0

feat: migrated vue 2 backwards compatibility for @vue-flow/background

michealroberts force-pushed the refactor/backward/vue-2-backwards-compat branch from 7f756d7 to 95d49a0 Compare November 10, 2023 12:09

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 12:09 Inactive

bcakmakoglu reviewed Nov 10, 2023

View reviewed changes

fix: remove accidental push

f873924

fix: remove accidental push

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 12:16 Inactive

fix: ensure background depends on core package

5a4ce17

vercel bot temporarily deployed to Preview – vue-flow-typedoc November 10, 2023 12:22 Inactive

michealroberts mentioned this pull request Nov 12, 2023

[ADVICE] default as import of defineComponent() has broken typing vuejs/language-tools#3726

Closed

michealroberts closed this Nov 17, 2023

		import UserSelection from '../../components/UserSelection/UserSelection.vue'
		import NodesSelection from '../../components/NodesSelection/NodesSelection.vue'

feat: added vue 2 backwards compatibility for @vue-flow/background #1188

feat: added vue 2 backwards compatibility for @vue-flow/background #1188

Conversation

michealroberts commented Nov 9, 2023 • edited Loading

Description

Testing 🔧

changeset-bot bot commented Nov 9, 2023 • edited Loading

🦋 Changeset detected

vercel bot commented Nov 9, 2023 • edited Loading

vercel bot commented Nov 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcakmakoglu Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcakmakoglu commented Nov 9, 2023

michealroberts commented Nov 9, 2023

bcakmakoglu commented Nov 9, 2023

bcakmakoglu commented Nov 9, 2023

michealroberts commented Nov 9, 2023

michealroberts commented Nov 9, 2023 • edited Loading

bcakmakoglu commented Nov 9, 2023

bcakmakoglu commented Nov 9, 2023

michealroberts commented Nov 9, 2023 • edited Loading

bcakmakoglu commented Nov 9, 2023

bcakmakoglu commented Nov 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcakmakoglu Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcakmakoglu commented Nov 10, 2023 • edited Loading

michealroberts commented Nov 10, 2023

bcakmakoglu commented Nov 10, 2023

bcakmakoglu commented Nov 10, 2023

Choose a reason for hiding this comment

michealroberts commented Nov 10, 2023

Choose a reason for hiding this comment

bcakmakoglu commented Nov 10, 2023

michealroberts commented Nov 10, 2023

bcakmakoglu commented Nov 10, 2023

michealroberts commented Nov 17, 2023

michealroberts commented Nov 9, 2023 •

edited

Loading

changeset-bot bot commented Nov 9, 2023 •

edited

Loading

vercel bot commented Nov 9, 2023 •

edited

Loading

bcakmakoglu Nov 9, 2023 •

edited

Loading

michealroberts commented Nov 9, 2023 •

edited

Loading

michealroberts commented Nov 9, 2023 •

edited

Loading

bcakmakoglu Nov 9, 2023 •

edited

Loading

bcakmakoglu commented Nov 10, 2023 •

edited

Loading