⚡️ Introduce FE external hooks #1332
Conversation
ahsanv
force-pushed
the
av/external-hooks-fe
branch
2 times, most recently
from
996436f
to
679bd11
Compare
| @@ -24,6 +24,7 @@ | |||
|
|
|||
| import { genericHelpers } from '@/components/mixins/genericHelpers'; | |||
| import { restApi } from '@/components/mixins/restApi'; | |||
| import { externalHooks } from '@/components/mixins/externalHooks'; | |||
| @@ -37,6 +38,7 @@ export default mixins( | |||
| restApi, | |||
| showMessage, | |||
| workflowHelpers, | |||
| externalHooks, | |||
| @@ -0,0 +1,38 @@ | |||
| import Vue from "vue"; | |||
| import { IExternalHooks } from "@/Interface"; | |||
| import { IDataObject } from "n8n-workflow"; | |||
There was a problem hiding this comment.
Uses double instead of single quotes here and multiple other locations.
| metadata?: IDataObject, | ||
| ) { | ||
| // @ts-ignore | ||
| if (!window.externalHooks) { |
There was a problem hiding this comment.
Lets maybe use a more specific name like n8nExternalHooks.
packages/editor-ui/src/Interface.ts
Outdated
| @@ -117,6 +117,10 @@ export interface INodeTypesMaxCount { | |||
| }; | |||
| } | |||
|
|
|||
| export interface IExternalHooks { | |||
| onExternalHookEvent(eventName: string, metadata?: IDataObject): void; | |||
There was a problem hiding this comment.
The name is misleading. It sounds like it would execute something when an external hook event gets called but it does actually the opposite.
| externalHooks(): IExternalHooks { | ||
| return { | ||
| onExternalHookEvent: (eventName: string, metadata?: IDataObject): void => { | ||
| runExternalHook(eventName, this.$store.state, metadata); |
There was a problem hiding this comment.
Is there a reason why we provide the store.state? Should it not rather be the store itself to be able to use getters and commit? Or is it not simply better to use .call(this, ... to stay flexible?
| @@ -118,6 +118,7 @@ import { genericHelpers } from '@/components/mixins/genericHelpers'; | |||
| import { mouseSelect } from '@/components/mixins/mouseSelect'; | |||
| import { moveNodeWorkflow } from '@/components/mixins/moveNodeWorkflow'; | |||
| import { restApi } from '@/components/mixins/restApi'; | |||
| import { externalHooks } from '@/components/mixins/externalHooks'; | |||
| @@ -172,6 +173,7 @@ export default mixins( | |||
| titleChange, | |||
| workflowHelpers, | |||
| workflowRun, | |||
| externalHooks, | |||
There was a problem hiding this comment.
@janober could probably setup husky pre-commit hooks to handle sorting imports and possibly failing commits until things that cannot be auto fixed (like using camelCase) are fixed manually by the committer
There was a problem hiding this comment.
For the backend modules, the rules are already in place and everything got cleaned up and is fine. On the frontend, however, we did not look into that yet. So agree we probably should do that soon.
| @@ -129,6 +131,7 @@ export default mixins( | |||
|
|
|||
| if (newActiveState === true) { | |||
| this.$store.commit('setWorkflowActive', this.workflowId); | |||
| this.externalHooks().onExternalHookEvent('workflow.onWorkflowActivated', { workflow_id: this.workflowId, workflow_name: this.$store.getters.workflowName }); | |||
There was a problem hiding this comment.
We use everywhere camelCase in n8n, so should also be the case here.
| @@ -2033,6 +2039,8 @@ export default mixins( | |||
| } | |||
| this.stopLoading(); | |||
| }); | |||
|
|
|||
| this.externalHooks().onExternalHookEvent('nodeView.mounted', this.$store.state); | |||
There was a problem hiding this comment.
Providing store.state should not be needed.
ahsanv
force-pushed
the
av/external-hooks-fe
branch
from
71b8d5a
to
8f08377
Compare
|
Thanks a lot. Got merged. |
|
Got released with n8n@0.103.1 |
No description provided.