Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Feb 12, 2025

Summary

solves an incompatibility issue when the providing app registers their code from an incompatible nextcloud-vue version.

Also changes and clarifies WorkflowEngine API. This is necessary to stay compatible with the original way, but also promotes usage of the originally declared but never used "component" attribute on registration.

TODO

  • ...

Checklist

@blizzz
Copy link
Member Author

blizzz commented Feb 12, 2025

/backport to stable31

@blizzz
Copy link
Member Author

blizzz commented Feb 12, 2025

/backport to stable30

@blizzz
Copy link
Member Author

blizzz commented Feb 12, 2025

Might need to do the same stuff for registering Checks, need to verify.

@blizzz blizzz force-pushed the fix/49909/workflow-vue-compat branch 2 times, most recently from 275ddb9 to 9ac3a96 Compare February 12, 2025 21:13
Comment on lines 137 to 141
const View = this.operation.component
this.component = new View()
this.component.$mount(this.$refs.operationComponent)
this.component.$on('input', this.updateOperation)
this.component.$props.value = this.rule.operation
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note, such an approach only works in Vue 2 (which is EOL since 31 Dec 2023).
In the current Vue:

  • Component cannot be used as a constructor with new component(). Only app can, created with createApp(component). And this createApp must be from the same Vue, as component = same issue as in the past
  • $on was deprecated and has been removed

Let me think about a simple alternative...

Copy link
Contributor

Choose a reason for hiding this comment

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

Option 1: WebComponents

// In an app defining operation

import Vue from 'vue'
import wrap from '@vue/web-component-wrapper'

// Wrap Vue component into custom HTML Element
const CustomOperationElement = wrap(Vue, OperationComponent)
// Register with some UNIQ name
window.customElements.define('custom-operation', CustomElement)

// In Vue 2, wrap doesn's support disabling shadow :(
// Disable with a hack
Object.defineProperty(CustomElement.prototype, 'attachShadow', { value() { return this } })
Object.defineProperty(CustomElement.prototype, 'shadowRoot', { get() { return this } })

// ---

// In workflowengine
// <component :is="'custom-operation'" />

Copy link
Contributor

Choose a reason for hiding this comment

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

Option 2: provide a register method so CustomOperation developers can manually mount the app.

We do it in Viewer.

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @susnux

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the webcomponent approach, but we should be consistent and either also do so on viewer or use option 2

Copy link
Contributor

Choose a reason for hiding this comment

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

@juliusknorr I mean for the next version nextcloud/viewer#2395

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with both - but I agree the Webcomponent looks more elegant.
What would the event listener in the workflow engine look like? Could we do something like:

<component :is="'custom-operation'"  @input="handleInput" />

or even use v-model as we currently do?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vue/web-component-wrapper docs say:

Custom events emitted on the inner Vue component are dispatched on the custom element as a CustomEvent. Additional arguments passed to $emit will be exposed as an Array as event.detail.

Sounds like event handing should work.

Copy link
Contributor

Choose a reason for hiding this comment

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

only works in Vue 2 (which is EOL since 31 Dec 2023).

Just to clarify. Even if the server isn't about to be migrated to Vue 3 very soon, this limitation won't allow other apps to use Vue 3 and have integration with the workflow engine

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with both - but I agree the Webcomponent looks more elegant. What would the event listener in the workflow engine look like? Could we do something like:

<component :is="'custom-operation'"  @input="handleInput" />

or even use v-model as we currently do?

Sounds like event handing should work.

Yeah, it's a little bit more complicated. Events are emitted as native custom events. So:

  • You don't have the input value as the event object, but a CustomEvent object. The value is $event.detail[0]
  • You cannot use v-model (for the reason above)
  • You cannot use input as the custom event name, until you stop bubbling native input event

So I'd suggest to use modelValue + update:modelValue event to not conflict with natives.

<component
  :is="operationElementName"
  :model-value="inputValue"
  @update:model-value="inputValue = $event.detail[0]"
/>

https://stackblitz.com/edit/vitejs-vite-yyhbtzus

@blizzz
Copy link
Member Author

blizzz commented Feb 19, 2025

Thank you all for the input here!

I pushed the suggested changes hereto as well as to Talk's flow.

I could not get the :model-value thing to work, I always ended up with Property or method "inputValue" is not defined on the instance but referenced during render. .

Also, the @update event did not trigger, got it working with @input only – which is also emitted by Talk`s flow. Less changes to existing code the better, it's bad enough that stuff is breaking and maintaining backwards compatibility is a goal as well. How much of a problem is that?

But I might have been holding it wrong, this is far from my area of confidence 😅

Didn't do yet:

  • satisfying linter
  • squash and rephrase commit
  • update documentation once more

@ShGKme
Copy link
Contributor

ShGKme commented Feb 19, 2025

@blizzz I'll have a look in the morning. I must work, I checked :D

As an alternative, we can also define both options:

  • component (componentVue2) - for easy migration for existing apps with Vue.extend
  • element - for CustomElement (WebComponent) with Vue 3 compatibility

Then we have a quick and simple fix for the release + a better solution for the future

@blizzz
Copy link
Member Author

blizzz commented Feb 20, 2025

component (componentVue2) - for easy migration for existing apps with Vue.extend

if the app has to be touched anyway – it has – there is no reason to have a bridge solution there (provided WebComponents works). less is more.

Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

I did not take a look at the entire file - just some had a brief glance. Hope the pointers are useful. Happy to have a call to follow up.

@blizzz blizzz force-pushed the fix/49909/workflow-vue-compat branch from 3a2794a to 33a8223 Compare March 6, 2025 13:55
@blizzz blizzz requested a review from a team as a code owner March 6, 2025 13:55
@blizzz blizzz requested review from Pytal, artonge and skjnldsv and removed request for a team March 6, 2025 13:55
@blizzz blizzz force-pushed the fix/49909/workflow-vue-compat branch from 33a8223 to d508de1 Compare March 6, 2025 14:27
@blizzz blizzz changed the title fix(workflowengine): require a VueConstructor object as operation plugin fix(workflowengine): require a web component as operation plugin Mar 6, 2025
@blizzz
Copy link
Member Author

blizzz commented Mar 6, 2025

Updated, if I recall correctly @max-nextcloud, for a minimal fix we want to stay like this.

I still need to

  • push the changes to the spreed PR
  • check the Check Plugin, i expect it needs similar changes

@blizzz
Copy link
Member Author

blizzz commented Apr 3, 2025

/backport to stable30

This was referenced Apr 3, 2025
@blizzz
Copy link
Member Author

blizzz commented Apr 3, 2025

Failing test not related.

@blizzz blizzz disabled auto-merge April 3, 2025 11:20
@blizzz blizzz merged commit 8079584 into master Apr 3, 2025
128 of 132 checks passed
@blizzz blizzz deleted the fix/49909/workflow-vue-compat branch April 3, 2025 11:20
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 📝 Office team Apr 3, 2025
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
susnux added a commit to nextcloud/files_downloadlimit that referenced this pull request Sep 1, 2025
* Based on nextcloud/server#50783
* For #514
  * Server companion: nextcloud/server#54788
  * Library companion: nextcloud-libraries/nextcloud-sharing#95

We  need to provide an API that does not rely on Vue.
As Vue components **must not** be shared between different apps.

Signed-off-by: Ferdinand Thiessen <[email protected]>
CarlSchwan pushed a commit to nextcloud/files_downloadlimit that referenced this pull request Sep 18, 2025
* Based on nextcloud/server#50783
* For #514
  * Server companion: nextcloud/server#54788
  * Library companion: nextcloud-libraries/nextcloud-sharing#95

We  need to provide an API that does not rely on Vue.
As Vue components **must not** be shared between different apps.

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
CarlSchwan pushed a commit to nextcloud/files_downloadlimit that referenced this pull request Sep 19, 2025
* Based on nextcloud/server#50783
* For #514
  * Server companion: nextcloud/server#54788
  * Library companion: nextcloud-libraries/nextcloud-sharing#95

We  need to provide an API that does not rely on Vue.
As Vue components **must not** be shared between different apps.

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
backportbot bot pushed a commit to nextcloud/files_downloadlimit that referenced this pull request Sep 19, 2025
* Based on nextcloud/server#50783

* For #514

* Server companion: nextcloud/server#54788

* Library companion: nextcloud-libraries/nextcloud-sharing#95

We  need to provide an API that does not rely on Vue.

As Vue components **must not** be shared between different apps.

Signed-off-by: Ferdinand Thiessen <[email protected]>

Signed-off-by: Carl Schwan <[email protected]>

[skip ci]
susnux added a commit to nextcloud/files_downloadlimit that referenced this pull request Sep 24, 2025
* Based on nextcloud/server#50783
* For #514
  * Server companion: nextcloud/server#54788
  * Library companion: nextcloud-libraries/nextcloud-sharing#95

We  need to provide an API that does not rely on Vue.
As Vue components **must not** be shared between different apps.

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
susnux added a commit to nextcloud/files_downloadlimit that referenced this pull request Sep 24, 2025
* Based on nextcloud/server#50783
* For #514
  * Server companion: nextcloud/server#54788
  * Library companion: nextcloud-libraries/nextcloud-sharing#95

We  need to provide an API that does not rely on Vue.
As Vue components **must not** be shared between different apps.

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

8 participants