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.

Sign up for GitHub

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

fix(workflowengine): require a VueConstructor object as operation plugin #50783

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(workflowengine): require a VueConstructor object as operation plugin #50783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion apps/workflowengine/src/components/Rule.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
<div class="flow-icon icon-confirm" />
<div class="action">
<Operation :operation="operation" :colored="false">
<div v-if="operation.component"
ref="operationComponent" />
<component :is="operation.options"
v-if="operation.options"
v-else-if="operation.options"
v-model="rule.operation"
@input="updateOperation" />
</Operation>
Expand Down Expand Up @@ -98,9 +100,13 @@ export default {
error: null,
dirty: this.rule.id < 0,
originalRule: null,
component: null,
}
},
computed: {
/**
* @return {OperatorPlugin}
*/
operation() {
return this.$store.getters.getOperationForRule(this.rule)
},
Expand All @@ -126,10 +132,23 @@ export default {
},
mounted() {
this.originalRule = JSON.parse(JSON.stringify(this.rule))

if (this.operation?.component) {
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
Comment on lines +137 to +141
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

} else if (this.operation?.options) {
// keeping this in an else for apps that try to be backwards compatible and may ship both
// to be removed in 03/2028
console.warn('Developer warning: `OperatorPlugin.options` is deprecated. Use `OperatorPlugin.components` instead.')
}
},
methods: {
async updateOperation(operation) {
this.$set(this.rule, 'operation', operation)
this.component.$props.value = operation
await this.updateRule()
},
validate(/* state */) {
Expand Down
4 changes: 4 additions & 0 deletions apps/workflowengine/src/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ const store = new Store({
return rule1.id - rule2.id || rule2.class - rule1.class
})
},
/**
* @param state
* @return {OperatorPlugin}
*/
getOperationForRule(state) {
return (rule) => state.operations[rule.class]
},
Expand Down
11 changes: 9 additions & 2 deletions apps/workflowengine/src/workflowengine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,17 @@ import ShippedChecks from './components/Checks/index.js'
* @property {string} id - The PHP class name of the check
* @property {string} operation - Default value for the operation field
* @property {string} color - Custom color code to be applied for the operator selector
* @property {Vue} component - A vue component to handle the rendering of options
* @property {object} [options] - Deprecated: **Use `component` instead**
*
* A vue component to handle the rendering of options.
* The component should handle the v-model directive properly,
* so it needs a value property to receive data and emit an input
* event once the data has changed
* event once the data has changed.
*
* Will be removed in 03/2028.
* @property {VueConstructor} [component] - A vue constructor as returned by `Vue.extend()`.
* It has to emit the `$input` event when a value was changed.
* The `value` property will be set initially with the rule operation value.
*/

/**
Expand Down
4 changes: 2 additions & 2 deletions dist/workflowengine-workflowengine.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/workflowengine-workflowengine.js.map
View file
Open in desktop

Large diffs are not rendered by default.

Loading