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

Circular dependency between @nextcloud/vue and @nextcloud/vue-richtext #3812

Closed
raimund-schluessler opened this issue Feb 23, 2023 · 5 comments · Fixed by #3841
Closed

Circular dependency between @nextcloud/vue and @nextcloud/vue-richtext #3812

raimund-schluessler opened this issue Feb 23, 2023 · 5 comments · Fixed by #3841
Labels
question Further information is requested technical debt

Comments

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Feb 23, 2023

With #3708 we got into the unfortunate situation of a circular dependency between @nextcloud/vue and @nextcloud/vue-richtext. @nextcloud/vue now requires @nextcloud/vue-richtext which requires @nextcloud/vue which requires @nextcloud/vue-richtext which ..., you see what I mean 🙈

As long as the two packages release only compatible minor/patch versions, this is not a critical problem. But it is already cumbersome, because where do you stop updating if one package releases an update?

However, this situation becomes a critically blocking dead-lock, if we aim for a breaking major release. This will be the case with the migration to vue3 in #3692. In order to release a vue3 compatible version of @nextcloud/vue, we need a vue3 compatible version of @nextcloud/vue-richtext, which needs a vue3 compatible version of @nextcloud/vue, which... oh well. This won't work.

So, I really think we need to resolve this circular dependency. My proposed solution is to merge @nextcloud/vue-richtext into @nextcloud/vue. Since they anyway pull in each other, there is not much won by maintaining two separate packages/repositories. So I think that's a good solution. Plus, it's less effort to maintain a single package over two packages. We went the same way with @nextcloud/vue-dashboard and it worked fine.

What do you think about that @nextcloud/vuejs and especially @julien-nc, @mejo- @juliushaertl @skjnldsv?

@raimund-schluessler raimund-schluessler added question Further information is requested technical debt labels Feb 23, 2023
@raimund-schluessler raimund-schluessler changed the title Circular dependency between @nextcloud/vue and @nextcloud/vue-richttext Circular dependency between @nextcloud/vue and @nextcloud/vue-richtext Feb 23, 2023
@mejo-
Copy link
Contributor

mejo- commented Feb 23, 2023

Thanks for noticing @raimund-schluessler, good catch!

My proposed solution is to merge @nextcloud/vue-richtext into @nextcloud/vue. Since they anyway pull in each other, there is not much won by maintaining two separate packages/repositories.

👍 from me for that proposal.

@st3iny
Copy link
Contributor

st3iny commented Feb 23, 2023

Sounds good to me. Maintaining one less library is always a great thing.

However, I'm not familiar with the vue-richtext library so I don't know if there might be a good reason for the split.

@juliushaertl
Copy link
Contributor

I've actually also been thinking about moving vue-richtext to @nextcloud/vuewith @julien-nc. In the beginning the package was generic so that it didn't have any nextcloud related code, that is why I started it as a separate one, but I think it might be the best to move the code over these days.

@raimund-schluessler
Copy link
Contributor Author

Nice. I will have a look how to merge them.

@susnux
Copy link
Contributor

susnux commented Feb 23, 2023

From the user perspective (when developing NC apps) it also was not obvious why this are two different libraries.
So I think this also makes sense from that point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants