fix(workflowengine): require a web component as operation plugin

[blizzz]

• Contributes to fixing [Bug]: Flow "write to a conversation" broken. #49909
• Background: vue__WEBPACK_IMPORTED_MODULE_0__.getCurrentInstance() is null since 8.20  nextcloud-libraries/nextcloud-vue#6360
• Talk counterpart in fix(Flow): adapt to registration fixes at workflowengine spreed#14367

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

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[blizzz]

/backport to stable31

[blizzz]

/backport to stable30

[blizzz]

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

[ShGKme]

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...

[ShGKme]

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'" />

[ShGKme]

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

We do it in Viewer.

[ShGKme]

cc @susnux

[susnux]

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

[susnux]

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

[max-nextcloud]

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?

[max-nextcloud]

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

[ShGKme]

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

[ShGKme]

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]

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]

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

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.

[max-nextcloud]

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]

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

[max-nextcloud]

As a cleanup / follow up:

As far as i can tell this mixin is only used in two components:
Checks/RequestURL.vue
and
Checks/RequestUserAgent.vue

They both define the newValue in data as a string. So I think it would make sense to remove this definition here or revert it to string and remove it in the components making use of the mixin (or remove the mixin alltogether and add the watch and methods to the two remaining components.

[blizzz]

I was told mixins turned to be out bad and should not be used anymore. I had a case where they did not work as they were announced. So, follow up should simply eliminate it and move what is needed to those Checks.

[blizzz]

/compile amend

[blizzz]

/backport to stable31

[blizzz]

/backport to stable30

[blizzz]

Failing test not related.