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

Update @nextcloud/vue-richtext to avoid circular dependency issue #3850

Closed
wants to merge 2 commits into from

Conversation

julien-nc
Copy link
Member

This issue basically makes it impossible to use @nextcloud/vue-richtext in a script which also imports @nextcloud/vue >= 7.0.0 (when we introduced the circular dependency).

@julien-nc julien-nc force-pushed the fix/noid/bump-vue-richtext branch from baef596 to 778ca94 Compare February 28, 2023 16:24
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc julien-nc force-pushed the fix/noid/bump-vue-richtext branch from 0de8fe4 to 0e06c25 Compare February 28, 2023 16:58
@julien-nc
Copy link
Member Author

/compile amend

@julien-nc
Copy link
Member Author

No clue why it fails.

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Feb 28, 2023

https://github.com/nextcloud/text/actions/runs/4295726255/jobs/7486535732 says:

ERROR in ./node_modules/@nextcloud/vue-richtext/dist/index.js 16072:15-67
Module not found: Error: Can't resolve '@nextcloud/calendar-js' in '/home/runner/work/text/text/node_modules/@nextcloud/vue-richtext/dist'

and

ERROR in ./node_modules/@nextcloud/vue-richtext/dist/index.js 16775:54-79
Module not found: Error: Can't resolve 'linkify-string' in '/home/runner/work/text/text/node_modules/@nextcloud/vue-richtext/dist'
resolve 'linkify-string' in '/home/runner/work/text/text/node_modules/@nextcloud/vue-richtext/dist'

I suspect this marks @nextcloud/calendar-js and linkify-string as external:
https://github.com/nextcloud/vue-richtext/blob/master/vite.config.js#L18

So they need to be provided as peer dependencies by whatever uses vue-richtext.

@max-nextcloud
Copy link
Collaborator

I suspect this marks @nextcloud/calendar-js and linkify-string as external.

It does not. They are not even in the dependencies - quite the contrary. They are only dependencies of the @nextcloud/vue dependency which is explicitely not treated as external.

🤷

@juliusknorr
Copy link
Member

Even weirder that the build passes locally without any issues :/

@julien-nc
Copy link
Member Author

julien-nc commented Feb 28, 2023

I can reproduce locally. This happens when text is not inside a server clone. Apparently @nextcloud/calendar-js and linkify-string are obtained from the server's node_modules.

@juliusknorr
Copy link
Member

I think this is due to the bad tree shaking abilities of web pack. I currently see two workarounds:

  • We add both dependencies temporarily to text
  • Switching all imports in @nextcloud/vue-richtext to use the dist/ folder of @nextcloud/vue - This seems to work locally but I have no good explanation yet on what the difference is

@julien-nc
Copy link
Member Author

We can also wait for #3859 which is a better fix anyway.

@mejo-
Copy link
Member

mejo- commented Mar 2, 2023

Should be good to close now that #3859 landed, right?

@mejo- mejo- closed this Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants