feat(editor): Add several performance improvements when adding nodes in new canvas (no-changelog) (#10170)

This commit is contained in:
Alex Grozav 2024-07-25 15:26:50 +03:00 committed by GitHub
parent 520f2316d1
commit 112d6b883d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 170 additions and 105 deletions

View file

@ -5,6 +5,7 @@ import type { Workflow } from 'n8n-workflow';
import type { IWorkflowDb } from '@/Interface';
import { useCanvasMapping } from '@/composables/useCanvasMapping';
import type { EventBus } from 'n8n-design-system';
import { createEventBus } from 'n8n-design-system';
defineOptions({
inheritAttrs: false,
@ -21,6 +22,7 @@ const props = withDefaults(
}>(),
{
id: 'canvas',
eventBus: () => createEventBus(),
fallbackNodes: () => [],
},
);

View file

@ -24,6 +24,7 @@ import { useCredentialsStore } from '@/stores/credentials.store';
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import { telemetry } from '@/plugins/telemetry';
import { useClipboard } from '@/composables/useClipboard';
import { waitFor } from '@testing-library/vue';
vi.mock('vue-router', async (importOriginal) => {
const actual = await importOriginal<{}>();
@ -74,68 +75,89 @@ describe('useCanvasOperations', () => {
workflowsStore.resetWorkflow();
workflowsStore.resetState();
await workflowHelpers.initState(workflow);
workflowHelpers.initState(workflow);
canvasOperations = useCanvasOperations({ router, lastClickPosition });
vi.clearAllMocks();
});
describe('addNode', () => {
it('should throw error when node type does not exist', async () => {
vi.spyOn(nodeTypesStore, 'getNodeTypes').mockResolvedValue(undefined);
describe('requireNodeTypeDescription', () => {
it('should return node type description when type and version match', () => {
const type = 'testType';
const version = 1;
const expectedDescription = mockNodeTypeDescription({ name: type, version });
nodeTypesStore.setNodeTypes([expectedDescription]);
await expect(canvasOperations.addNode({ type: 'nonexistent' })).rejects.toThrow();
const result = canvasOperations.requireNodeTypeDescription(type, version);
expect(result).toBe(expectedDescription);
});
it('should create node with default version when version is undefined', async () => {
nodeTypesStore.setNodeTypes([mockNodeTypeDescription({ name: 'type' })]);
it('should throw an error when node type does not exist', () => {
const type = 'nonexistentType';
const result = await canvasOperations.addNode({
name: 'example',
type: 'type',
});
expect(() => {
canvasOperations.requireNodeTypeDescription(type);
}).toThrow();
});
it('should return node type description when only type is provided and it exists', () => {
const type = 'testTypeWithoutVersion';
const expectedDescription = mockNodeTypeDescription({ name: type });
nodeTypesStore.setNodeTypes([expectedDescription]);
const result = canvasOperations.requireNodeTypeDescription(type);
expect(result).toBe(expectedDescription);
});
});
describe('addNode', () => {
it('should create node with default version when version is undefined', () => {
const result = canvasOperations.addNode(
{
name: 'example',
type: 'type',
typeVersion: 1,
},
mockNodeTypeDescription({ name: 'type' }),
);
expect(result.typeVersion).toBe(1);
});
it('should create node with last version when version is an array', async () => {
nodeTypesStore.setNodeTypes([mockNodeTypeDescription({ name: 'type', version: [1, 2] })]);
const result = await canvasOperations.addNode({
type: 'type',
});
expect(result.typeVersion).toBe(2);
});
it('should create node with default position when position is not provided', async () => {
nodeTypesStore.setNodeTypes([mockNodeTypeDescription({ name: 'type' })]);
const result = await canvasOperations.addNode({
type: 'type',
});
it('should create node with default position when position is not provided', () => {
const result = canvasOperations.addNode(
{
type: 'type',
typeVersion: 1,
},
mockNodeTypeDescription({ name: 'type' }),
);
expect(result.position).toEqual([460, 460]); // Default last click position
});
it('should create node with provided position when position is provided', async () => {
nodeTypesStore.setNodeTypes([mockNodeTypeDescription({ name: 'type' })]);
const result = await canvasOperations.addNode({
type: 'type',
position: [20, 20],
});
it('should create node with provided position when position is provided', () => {
const result = canvasOperations.addNode(
{
type: 'type',
typeVersion: 1,
position: [20, 20],
},
mockNodeTypeDescription({ name: 'type' }),
);
expect(result.position).toEqual([20, 20]);
});
it('should create node with default credentials when only one credential is available', async () => {
it('should create node with default credentials when only one credential is available', () => {
const credential = mock<ICredentialsResponse>({ id: '1', name: 'cred', type: 'cred' });
const nodeTypeName = 'type';
nodeTypesStore.setNodeTypes([
mockNodeTypeDescription({ name: nodeTypeName, credentials: [{ name: credential.name }] }),
]);
const nodeTypeDescription = mockNodeTypeDescription({
name: nodeTypeName,
credentials: [{ name: credential.name }],
});
credentialsStore.addCredentials([credential]);
@ -144,24 +166,25 @@ describe('useCanvasOperations', () => {
credential,
]);
const result = await canvasOperations.addNode({
type: nodeTypeName,
});
const result = canvasOperations.addNode(
{
type: nodeTypeName,
typeVersion: 1,
},
nodeTypeDescription,
);
expect(result.credentials).toEqual({ [credential.name]: { id: '1', name: credential.name } });
});
it('should not assign credentials when multiple credentials are available', async () => {
it('should not assign credentials when multiple credentials are available', () => {
const credentialA = mock<ICredentialsResponse>({ id: '1', name: 'credA', type: 'cred' });
const credentialB = mock<ICredentialsResponse>({ id: '1', name: 'credB', type: 'cred' });
const nodeTypeName = 'type';
nodeTypesStore.setNodeTypes([
mockNodeTypeDescription({
name: nodeTypeName,
credentials: [{ name: credentialA.name }, { name: credentialB.name }],
}),
]);
const nodeTypeDescription = mockNodeTypeDescription({
name: nodeTypeName,
credentials: [{ name: credentialA.name }, { name: credentialB.name }],
});
// @ts-expect-error Known pinia issue when spying on store getters
vi.spyOn(credentialsStore, 'getUsableCredentialByType', 'get').mockReturnValue(() => [
@ -169,24 +192,30 @@ describe('useCanvasOperations', () => {
credentialB,
]);
const result = await canvasOperations.addNode({
type: 'type',
});
const result = canvasOperations.addNode(
{
type: 'type',
typeVersion: 1,
},
nodeTypeDescription,
);
expect(result.credentials).toBeUndefined();
});
it('should open NDV when specified', async () => {
nodeTypesStore.setNodeTypes([mockNodeTypeDescription({ name: 'type' })]);
const nodeTypeDescription = mockNodeTypeDescription({ name: 'type' });
await canvasOperations.addNode(
canvasOperations.addNode(
{
type: 'type',
typeVersion: 1,
name: 'Test Name',
},
nodeTypeDescription,
{ openNDV: true },
);
expect(ndvStore.activeNodeName).toBe('Test Name');
await waitFor(() => expect(ndvStore.activeNodeName).toBe('Test Name'));
});
});
@ -719,7 +748,7 @@ describe('useCanvasOperations', () => {
const addConnectionSpy = vi.spyOn(workflowsStore, 'addConnection');
await canvasOperations.addConnections(connections);
canvasOperations.addConnections(connections);
expect(addConnectionSpy).toHaveBeenCalledWith({
connection: [

View file

@ -97,6 +97,10 @@ type AddNodeData = Partial<INodeUi> & {
type: string;
};
type AddNodeDataWithTypeVersion = AddNodeData & {
typeVersion: INodeUi['typeVersion'];
};
type AddNodeOptions = {
dragAndDrop?: boolean;
openNDV?: boolean;
@ -413,6 +417,18 @@ export function useCanvasOperations({
historyStore.stopRecordingUndo();
}
function requireNodeTypeDescription(type: INodeUi['type'], version?: INodeUi['typeVersion']) {
const nodeTypeDescription = nodeTypesStore.getNodeType(type, version);
if (!nodeTypeDescription) {
throw new Error(
i18n.baseText('nodeView.showMessage.addNodeButton.message', {
interpolate: { nodeTypeName: type },
}),
);
}
return nodeTypeDescription;
}
async function addNodes(
nodes: AddedNodesAndConnections['nodes'],
options: {
@ -420,6 +436,7 @@ export function useCanvasOperations({
position?: XYPosition;
trackHistory?: boolean;
trackBulk?: boolean;
keepPristine?: boolean;
} = {},
) {
let insertPosition = options.position;
@ -429,16 +446,32 @@ export function useCanvasOperations({
historyStore.startRecordingUndo();
}
for (const nodeAddData of nodes) {
const nodesWithTypeVersion = nodes.map((node) => {
const typeVersion = resolveNodeVersion(requireNodeTypeDescription(node.type));
return {
...node,
typeVersion,
};
});
await loadNodeTypesProperties(nodesWithTypeVersion);
if (options.trackBulk) {
historyStore.startRecordingUndo();
}
for (const nodeAddData of nodesWithTypeVersion) {
const { isAutoAdd, openDetail: openNDV, ...node } = nodeAddData;
const position = node.position ?? insertPosition;
const nodeTypeDescription = requireNodeTypeDescription(node.type, node.typeVersion);
try {
lastAddedNode = await addNode(
lastAddedNode = addNode(
{
...node,
position,
},
nodeTypeDescription,
{
...options,
openNDV,
@ -459,13 +492,16 @@ export function useCanvasOperations({
}
if (lastAddedNode) {
// @TODO Figure out what this does and why it's needed
updatePositionForNodeWithMultipleInputs(lastAddedNode);
}
if (options.trackBulk) {
historyStore.stopRecordingUndo();
}
if (!options.keepPristine) {
uiStore.stateIsDirty = true;
}
}
function updatePositionForNodeWithMultipleInputs(node: INodeUi) {
@ -484,16 +520,11 @@ export function useCanvasOperations({
}
}
async function addNode(node: AddNodeData, options: AddNodeOptions = {}): Promise<INodeUi> {
const nodeTypeDescription = nodeTypesStore.getNodeType(node.type);
if (!nodeTypeDescription) {
throw new Error(
i18n.baseText('nodeView.showMessage.addNodeButton.message', {
interpolate: { nodeTypeName: node.type },
}),
);
}
function addNode(
node: AddNodeDataWithTypeVersion,
nodeTypeDescription: INodeTypeDescription,
options: AddNodeOptions = {},
): INodeUi {
// Check if maximum allowed number of this type of node has been reached
if (
nodeTypeDescription.maxNodes !== undefined &&
@ -507,35 +538,32 @@ export function useCanvasOperations({
);
}
const nodeData = await resolveNodeData(node, nodeTypeDescription);
const nodeData = resolveNodeData(node, nodeTypeDescription);
if (!nodeData) {
throw new Error(i18n.baseText('nodeViewV2.showError.failedToCreateNode'));
}
historyStore.startRecordingUndo();
if (options.trackHistory) {
historyStore.pushCommandToUndo(new AddNodeCommand(nodeData));
}
workflowsStore.addNode(nodeData);
nodeHelpers.matchCredentials(nodeData);
if (!options.isAutoAdd) {
createConnectionToLastInteractedWithNode(nodeData, options);
}
void nextTick(() => {
if (options.trackHistory) {
historyStore.pushCommandToUndo(new AddNodeCommand(nodeData));
}
runAddNodeHooks(nodeData, options);
workflowsStore.addNode(nodeData);
historyStore.stopRecordingUndo();
if (!options.isAutoAdd) {
createConnectionToLastInteractedWithNode(nodeData, options);
}
runAddNodeHooks(nodeData, options);
if (options.openNDV) {
ndvStore.setActiveNodeName(nodeData.name);
}
});
workflowsStore.setNodePristine(nodeData.name, true);
uiStore.stateIsDirty = true;
if (options.openNDV) {
void nextTick(() => {
ndvStore.setActiveNodeName(nodeData.name);
});
}
return nodeData;
}
@ -661,11 +689,14 @@ export function useCanvasOperations({
/**
* Resolves the data for a new node
*/
async function resolveNodeData(node: AddNodeData, nodeTypeDescription: INodeTypeDescription) {
function resolveNodeData(
node: AddNodeDataWithTypeVersion,
nodeTypeDescription: INodeTypeDescription,
) {
const id = node.id ?? uuid();
const name = node.name ?? (nodeTypeDescription.defaults.name as string);
const type = nodeTypeDescription.name;
const typeVersion = resolveNodeVersion(nodeTypeDescription);
const typeVersion = node.typeVersion;
const position = resolveNodePosition(node as INodeUi, nodeTypeDescription);
const disabled = node.disabled ?? false;
const parameters = node.parameters ?? {};
@ -681,8 +712,6 @@ export function useCanvasOperations({
parameters,
};
await loadNodeTypesProperties([{ name: nodeData.type, version: nodeData.typeVersion }]);
resolveNodeParameters(nodeData);
resolveNodeCredentials(nodeData, nodeTypeDescription);
resolveNodeName(nodeData);
@ -691,7 +720,9 @@ export function useCanvasOperations({
return nodeData;
}
async function loadNodeTypesProperties(nodeInfos: INodeTypeNameVersion[]): Promise<void> {
async function loadNodeTypesProperties(
nodes: Array<Pick<INodeUi, 'type' | 'typeVersion'>>,
): Promise<void> {
const allNodeTypeDescriptions: INodeTypeDescription[] = nodeTypesStore.allNodeTypes;
const nodesToBeFetched: INodeTypeNameVersion[] = [];
@ -700,8 +731,8 @@ export function useCanvasOperations({
? nodeTypeDescription.version
: [nodeTypeDescription.version];
if (
!!nodeInfos.find(
(n) => n.name === nodeTypeDescription.name && nodeVersions.includes(n.version),
!!nodes.find(
(n) => n.type === nodeTypeDescription.name && nodeVersions.includes(n.typeVersion),
) &&
!nodeTypeDescription.hasOwnProperty('properties')
) {
@ -1154,7 +1185,7 @@ export function useCanvasOperations({
return false;
}
async function addConnections(connections: CanvasConnectionCreateData[] | CanvasConnection[]) {
function addConnections(connections: CanvasConnectionCreateData[] | CanvasConnection[]) {
for (const { source, target, data } of connections) {
createConnection({
source,
@ -1211,10 +1242,10 @@ export function useCanvasOperations({
async function initializeWorkspace(data: IWorkflowDb) {
// Set workflow data
await workflowHelpers.initState(data);
workflowHelpers.initState(data);
// Add nodes and connections
await addNodes(data.nodes);
await addNodes(data.nodes, { keepPristine: true });
workflowsStore.setConnections(data.connections);
}
@ -1383,7 +1414,7 @@ export function useCanvasOperations({
historyStore.startRecordingUndo();
await addNodes(Object.values(tempWorkflow.nodes));
await addConnections(
addConnections(
mapLegacyConnectionsToCanvasConnections(
tempWorkflow.connectionsBySourceNode,
Object.values(tempWorkflow.nodes),
@ -1668,6 +1699,7 @@ export function useCanvasOperations({
editableWorkflow,
editableWorkflowObject,
triggerNodes,
requireNodeTypeDescription,
addNodes,
addNode,
revertAddNode,

View file

@ -1055,7 +1055,7 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
}
}
async function initState(workflowData: IWorkflowDb): Promise<void> {
function initState(workflowData: IWorkflowDb) {
workflowsStore.addWorkflow(workflowData);
workflowsStore.setActive(workflowData.active || false);
workflowsStore.setWorkflowId(workflowData.id);

View file

@ -825,9 +825,11 @@ async function onAddNodesAndConnections(
};
});
await addConnections(mappedConnections);
addConnections(mappedConnections);
uiStore.resetLastInteractedWith();
void nextTick(() => {
uiStore.resetLastInteractedWith();
});
}
async function onRevertAddNode({ node }: { node: INodeUi }) {

View file

@ -1389,7 +1389,7 @@ export default defineComponent({
this.resetWorkspace();
await this.workflowHelpers.initState(workflow);
this.workflowHelpers.initState(workflow);
if (workflow.sharedWithProjects) {
this.workflowsEEStore.setWorkflowSharedWith({

View file

@ -125,7 +125,7 @@ async function fetchWorkflow() {
try {
await workflowsStore.fetchActiveWorkflows();
const data = await workflowsStore.fetchWorkflow(workflowId.value);
await workflowHelpers.initState(data);
workflowHelpers.initState(data);
await nodeHelpers.addNodes(data.nodes, data.connections);
} catch (error) {
toast.showError(error, i18n.baseText('nodeView.showError.openWorkflow.title'));