nextcloud-libraries / nextcloud-vue

🍱 Vue.js components for Nextcloud app development ✌ https://npmjs.org/@nextcloud/vue
https://nextcloud-vue-components.netlify.app/
Other
216 stars 87 forks source link

[next] Backports and Release #6106

Open raimund-schluessler opened 1 month ago

raimund-schluessler commented 1 month ago

@nextcloud-libraries/developers The vue3 next branch needs some love. There hasn't been a release since Jul 16, corresponding to nextcloud/vue@8.14.0 (we are at nextcloud/vue@8.19.0 already). This leads to a lot of missing features and fixes in next, which makes e.g. the Tasks app (to my knowledge still the only app using vue3 in production, unfortunately) not working perfectly with NC30.

Also, the backports from master to next are neglected badly.

In my opinion, this should be the next steps here:

raimund-schluessler commented 1 month ago

Personal side-note:

I understand that migrating such a huge project as Nextcloud from vue2 to vue3 is a pain, takes a lot of effort and is barely visible to users (i.e. cannot be sold as "we added a great new feature"). However, it was long clear that vue2 will reach EOL at the end of 2023 and even 9 months later Nextcloud still uses it and does not really take care of vue 3 (not even backporting, let alone making it the default target). While vue 2 hasn't had any major vulnerabilites (citing from Vue 2 EOL), I think it is a really bad practice to use unmaintained libraries, especially for a project like Nextcloud, that advocates for "keep your software up-to-date" with every own release. Furthermore, there has been a lot of community effort to push the vue 3 migration forward, nextcloud/vue has been completely migrated, and next and master were on feature parity, but it was not picked up by the company, unfortunately.

susnux commented 1 month ago

Do a nextcloud/vue@9.0.0-alpha.6

Sure, just prepare the PR :)

Backport every missing PR from master to next. Although that will be very many, it might be easier even to "just" merge master into next (although this will require a lot of manual work, either way).

I think everything that is needed is backported. A merge I think will no longer work, both branches are to diverged already.


In general after talking to many people at the Nextcloud Conf. / Contributors week (this week), the overall opinion seems to be:

  1. Ensure both v8 and v9 provide proper model values
    • for v8 add update:modelValue besides all input events and deprecate input where needed
    • for v8 add modelValue prop as duplicate of any prop that is considered the model value (often just value but there are also some places where a different name is used)
    • backport both to v9
    • remove the deprecated ones from v9
  2. Release a beta version (at this point we shall switch default branch)
  3. Release new versions of all component libraries (upload / dialogs / ...) built on top of the beta version
  4. Migrate some example app (e.g. Notifications or Activity) so people can take that as an example for their apps
  5. Start migrating Nextcloud apps
  6. Migrate Nextcloud core (this will very likely not happen for 31, but currently discussed for 32 after we have experience with how the app migrations worked out).

But overall Nextcloud employees will probably not have much time until 30.0.1 release - so I expect 1 and 2 not before that time. This is what I think was the consent of the discussions I had about that topic this week / at the conference.

dartcafe commented 1 month ago

I would like to second this and want to additionally point the interest to #5950 which is the last issue which keeps me from releasing Polls on pinia and vue3 to the beta state.

dartcafe commented 1 month ago

And the cherry on the cream would be a vue3 compatible version of @nextcloud/dialogs

raimund-schluessler commented 1 month ago

What would be the benefit of this approach outlined below?

  1. Ensure both v8 and v9 provide proper model values

    • for v8 add update:modelValue besides all input events and deprecate input where needed

    • for v8 add modelValue prop as duplicate of any prop that is considered the model value (often just value but there are also some places where a different name is used)

    • backport both to v9

    • remove the deprecated ones from v9

I think this just brings further effort for developing v9. v9 already uses modelValue everywhere (given we did not forget a place). Bringing modelValue additionally to v8 would of course allow app devs to switch to modelValue with v8 already. But the amount of work would be the same, and the move to v9 would still be a breaking change. So why make it more complicated than it is? Just move your app to modelValue when you migrate to v9.

susnux commented 1 month ago

Bringing modelValue additionally to v8 would of course allow app devs to switch to modelValue with v8 already. But the amount of work would be the same, and the move to v9 would still be a breaking change. So why make it more complicated than it is? Just move your app to modelValue when you migrate to v9.

The idea is to make co-maintaining v8 next to v9 (in apps) easier. If you use non-deprecated v8 props / events / functions you should ideally have only to switch the version number in the package.json. Because the vue3 specific changes are not really a problem on API level, there are some exceptions of course, but in general it should be possible to just keep everything working.

E.g. take NcInputField, there is no reason why you should have to change anything between v9 and v8.


I would like to second this and want to additionally point the interest to https://github.com/nextcloud-libraries/nextcloud-vue/issues/5950 which is the last issue which keeps me from releasing Polls on pinia and vue3 to the beta state.

Pinia works fine with Vue2 and Vue3 and also Vuex works with Vue3 and Vue2, meaning you could also just release Vue2 + pinia first or keep Vuex with Vue3 (while I do not recommend as Pinia replaces Vuex).

That issue is sadly a bit of technical dept we also discussed, we did the mistake in the past to wrap external libraries without proper abstraction. This means we can not simply switch the underlying library without making it breaking (proper way would be provide own abstractions of what is really needed and not pass everything down).

But I agree this is something to be fixed, while I also strongly recommend to use the native variant where possible.

dartcafe commented 1 month ago

Pinia works fine with Vue2 and Vue3 and also Vuex works with Vue3 and Vue2

I can confirm that, stating from the point an app migration. I went through the migration by first replacing vuex by pinia before i went through the complete migration (vue3 and ts). It was a good way to get familiar with the composition API and TS.

That issue is sadly a bit of technical dept we also discussed, we did the mistake in the past to wrap external libraries without proper abstraction. [...] But I agree this is something to be fixed, while I also strongly recommend to use the native variant where possible.

Switching to the native picker would mean a heavy pain, since I tweaked the picker to the behavior, it has now. From what I observed, it seems 'just' to be an issue of the thrown events, but I am not familiar with the code of the nextcloud implementation. It works as before, but the dialog is just closed unintentionally on every click.

But good to read it is on the table.

My pain is just, I have a completely migrated app, ready for beta status. But it can't be released, because this issue causes a huge UX impact.