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

[next] Backports and Release #6106

Open
raimund-schluessler opened this issue Sep 20, 2024 · 7 comments
Open

[next] Backports and Release #6106

raimund-schluessler opened this issue Sep 20, 2024 · 7 comments
Labels
1. to develop Accepted and waiting to be taken care of discussion Need advices, opinions or ideas on this topic technical debt vue 3 Related to the vue 3 migration
Milestone

Comments

@raimund-schluessler
Copy link
Contributor

@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:

  • Do a nextcloud/vue@9.0.0-alpha.6 immediately, to have available what has been backported at least.
  • 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).
  • New release of nextcloud/vue@9.0.0-alpha.7.
  • Make next the new default branch (name it main in light of new standard, and to reduce confusion) to force people to develop for vue3 and get it backported to stable8 (new name for master) if they need fixes (or features, which they shouldn't, honestly) for vue2.
@raimund-schluessler raimund-schluessler added 1. to develop Accepted and waiting to be taken care of discussion Need advices, opinions or ideas on this topic technical debt vue 3 Related to the vue 3 migration labels Sep 20, 2024
@raimund-schluessler raimund-schluessler added this to the 9.0.0 milestone Sep 20, 2024
@raimund-schluessler raimund-schluessler pinned this issue Sep 20, 2024
@raimund-schluessler
Copy link
Contributor Author

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
Copy link
Contributor

susnux commented Sep 20, 2024

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
  1. Release a beta version (at this point we shall switch default branch)
  2. Release new versions of all component libraries (upload / dialogs / ...) built on top of the beta version
  3. Migrate some example app (e.g. Notifications or Activity) so people can take that as an example for their apps
  4. Start migrating Nextcloud apps
  5. 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
Copy link
Contributor

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
Copy link
Contributor

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

@raimund-schluessler
Copy link
Contributor Author

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
Copy link
Contributor

susnux commented Sep 20, 2024

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 #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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of discussion Need advices, opinions or ideas on this topic technical debt vue 3 Related to the vue 3 migration
Projects
None yet
Development

No branches or pull requests

3 participants