Skip to content

Conversation

@max-nextcloud
Copy link
Collaborator

SyncService does way too many things.
Split it until we only have a minimal sync service for the provider left.

Preparation for moving to y-http provider.

@max-nextcloud max-nextcloud force-pushed the refactor/decompose-sync-service branch from b612877 to 8fff080 Compare July 1, 2025 13:49
@codecov
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 59.97426% with 311 lines in your changes missing coverage. Please review.

Project coverage is 59.43%. Comparing base (3bba1ba) to head (9ea9857).
Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
src/services/SyncService.ts 31.03% 180 Missing ⚠️
src/apis/Mention.ts 13.51% 32 Missing ⚠️
src/apis/Connect.ts 6.45% 29 Missing ⚠️
src/apis/Sync.ts 19.04% 17 Missing ⚠️
src/composables/useSyncService.ts 27.77% 13 Missing ⚠️
src/composables/useConnection.ts 68.57% 11 Missing ⚠️
src/components/Editor/GuestNameDialog.vue 0.00% 7 Missing ⚠️
src/services/SessionConnection.js 53.84% 6 Missing ⚠️
src/composables/useEditorMethods.ts 50.00% 5 Missing ⚠️
src/components/Menu/BaseActionEntry.js 0.00% 4 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7381      +/-   ##
==========================================
- Coverage   59.51%   59.43%   -0.09%     
==========================================
  Files         486      492       +6     
  Lines       37382    37347      -35     
  Branches     1097     1087      -10     
==========================================
- Hits        22247    22196      -51     
- Misses      15029    15045      +16     
  Partials      106      106              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@max-nextcloud max-nextcloud force-pushed the refactor/decompose-sync-service branch 25 times, most recently from 881eb9b to 639b1f4 Compare July 6, 2025 15:41
@max-nextcloud max-nextcloud force-pushed the refactor/decompose-sync-service branch 2 times, most recently from abf037d to 9556db5 Compare July 7, 2025 04:15
@max-nextcloud max-nextcloud force-pushed the refactor/decompose-sync-service branch 3 times, most recently from 6d006cf to 538d9ad Compare July 7, 2025 15:42
@max-nextcloud max-nextcloud force-pushed the refactor/decompose-sync-service branch from 538d9ad to c3230d3 Compare July 7, 2025 16:11
@max-nextcloud max-nextcloud marked this pull request as ready for review July 8, 2025 08:20
@max-nextcloud max-nextcloud force-pushed the refactor/decompose-sync-service branch from 13fa2af to 9ea9857 Compare July 8, 2025 08:43
Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work 🎉

@max-nextcloud max-nextcloud merged commit 4e2b9d5 into main Jul 8, 2025
63 of 66 checks passed
@max-nextcloud max-nextcloud deleted the refactor/decompose-sync-service branch July 8, 2025 10:24
@st3iny
Copy link
Member

st3iny commented Sep 9, 2025

@mejo @max-nextcloud

Commit 94f7619 seems to break mounting the text editor, for example, editing cards in Deck with links in it.

Related error messages:

Error: Failed to inject Editor
    setupEditor Description.vue:186
    mounted Description.vue:176
    addStack stack.js:31

TypeError: can't access property "registerPlugin", e is undefined
    setupEditor Description.vue:186
    mounted Description.vue:176
    addStack stack.js:31

[Vue warn]: provide() can only be used inside setup().

Problem

For some reason, provide is not working despite provideEditor() begin called in MarkdownContentEditor::setup(). Thus, injecting the editor later on always fails as it was never provided.

Refactoring the code to just use a variable makes it work again.

Temporary fix

diff --git a/src/composables/useEditor.ts b/src/composables/useEditor.ts
index e9d3fe439..28b2932d3 100644
--- a/src/composables/useEditor.ts
+++ b/src/composables/useEditor.ts
@@ -6,14 +6,15 @@
 import { Editor } from '@tiptap/core'
 import { inject, type InjectionKey, provide } from 'vue'
 
+let globalEditor: Editor | undefined
+
 export const editorKey = Symbol('tiptap:editor') as InjectionKey<Editor>
 export const provideEditor = (editor: Editor) => {
-	provide(editorKey, editor)
+	globalEditor = editor
 }
 export const useEditor = () => {
-	const editor = inject(editorKey)
-	if (!editor) {
+	if (!globalEditor) {
 		throw new Error('Failed to inject Editor')
 	}
-	return { editor: editor as Editor }
+	return { editor: globalEditor! }
 }

@max-nextcloud
Copy link
Collaborator Author

@st3iny Thanks a lot for investigating!

I'd prefer to explicitly provide editor in the component that uses the composable instead - do you think that would work as well?

@st3iny
Copy link
Member

st3iny commented Sep 9, 2025

I'd prefer to explicitly provide editor in the component that uses the composable instead - do you think that would work as well?

I tried inlining the provideEditor() function and call provide(editorKey, editor) directly inside the setup function of MarkdownContentEditor but it still didn't work.

There is something wrong inside Vue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants