Skip to content

Commit

Permalink
fix(editor): Fix performance issue in credentials list (#10988)
Browse files Browse the repository at this point in the history
  • Loading branch information
elsmr authored Sep 27, 2024
1 parent d2bc076 commit 7073ec6
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 121 deletions.
42 changes: 30 additions & 12 deletions packages/core/bin/generate-ui-types
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,6 @@ function findReferencedMethods(obj, refs = {}, latestName = '') {
const loader = new PackageDirectoryLoader(packageDir);
await loader.loadAll();

const knownCredentials = loader.known.credentials;
const credentialTypes = Object.values(loader.credentialTypes).map((data) => {
const credentialType = data.type;
if (
knownCredentials[credentialType.name].supportedNodes?.length > 0 &&
credentialType.httpRequestNode
) {
credentialType.httpRequestNode.hidden = true;
}
return credentialType;
});

const loaderNodeTypes = Object.values(loader.nodeTypes);

const definedMethods = loaderNodeTypes.reduce((acc, cur) => {
Expand Down Expand Up @@ -76,6 +64,36 @@ function findReferencedMethods(obj, refs = {}, latestName = '') {
}),
);

const knownCredentials = loader.known.credentials;
const credentialTypes = Object.values(loader.credentialTypes).map((data) => {
const credentialType = data.type;
const supportedNodes = knownCredentials[credentialType.name].supportedNodes ?? [];
if (supportedNodes.length > 0 && credentialType.httpRequestNode) {
credentialType.httpRequestNode.hidden = true;
}

credentialType.supportedNodes = supportedNodes;

if (!credentialType.iconUrl && !credentialType.icon) {
for (const supportedNode of supportedNodes) {
const nodeType = loader.nodeTypes[supportedNode]?.type.description;

if (!nodeType) continue;
if (nodeType.icon) {
credentialType.icon = nodeType.icon;
credentialType.iconColor = nodeType.iconColor;
break;
}
if (nodeType.iconUrl) {
credentialType.iconUrl = nodeType.iconUrl;
break;
}
}
}

return credentialType;
});

const referencedMethods = findReferencedMethods(nodeTypes);

await Promise.all([
Expand Down
84 changes: 44 additions & 40 deletions packages/editor-ui/src/components/CredentialIcon.vue
Original file line number Diff line number Diff line change
@@ -1,50 +1,59 @@
<script setup lang="ts">
import { computed } from 'vue';
import { useCredentialsStore } from '@/stores/credentials.store';
import { useRootStore } from '@/stores/root.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import type { ICredentialType } from 'n8n-workflow';
import NodeIcon from '@/components/NodeIcon.vue';
import { getThemedValue } from '@/utils/nodeTypesUtils';
import { useRootStore } from '@/stores/root.store';
import { useUIStore } from '@/stores/ui.store';
import { getThemedValue } from '@/utils/nodeTypesUtils';
import { N8nNodeIcon } from 'n8n-design-system';
import type { ICredentialType } from 'n8n-workflow';
import { computed } from 'vue';
const props = defineProps<{
credentialTypeName: string | null;
}>();
const credentialsStore = useCredentialsStore();
const nodeTypesStore = useNodeTypesStore();
const rootStore = useRootStore();
const uiStore = useUIStore();
const nodeTypesStore = useNodeTypesStore();
const credentialWithIcon = computed(() => getCredentialWithIcon(props.credentialTypeName));
const filePath = computed(() => {
const themeIconUrl = getThemedValue(credentialWithIcon.value?.iconUrl, uiStore.appliedTheme);
const nodeBasedIconUrl = computed(() => {
const icon = getThemedValue(credentialWithIcon.value?.icon);
if (!icon?.startsWith('node:')) return null;
return nodeTypesStore.getNodeType(icon.replace('node:', ''))?.iconUrl;
});
const iconSource = computed(() => {
const themeIconUrl = getThemedValue(
nodeBasedIconUrl.value ?? credentialWithIcon.value?.iconUrl,
uiStore.appliedTheme,
);
if (!themeIconUrl) {
return null;
return undefined;
}
return rootStore.baseUrl + themeIconUrl;
});
const relevantNode = computed(() => {
const icon = credentialWithIcon.value?.icon;
if (typeof icon === 'string' && icon.startsWith('node:')) {
const nodeType = icon.replace('node:', '');
return nodeTypesStore.getNodeType(nodeType);
}
if (!props.credentialTypeName) {
return null;
}
const iconType = computed(() => {
if (iconSource.value) return 'file';
else if (iconName.value) return 'icon';
return 'unknown';
});
const nodesWithAccess = credentialsStore.getNodesWithAccess(props.credentialTypeName);
if (nodesWithAccess.length) {
return nodesWithAccess[0];
}
const iconName = computed(() => {
const icon = getThemedValue(credentialWithIcon.value?.icon, uiStore.appliedTheme);
if (!icon || !icon?.startsWith('fa:')) return undefined;
return icon.replace('fa:', '');
});
return null;
const iconColor = computed(() => {
const { iconColor: color } = credentialWithIcon.value ?? {};
if (!color) return undefined;
return `var(--color-node-icon-${color})`;
});
function getCredentialWithIcon(name: string | null): ICredentialType | null {
Expand All @@ -64,8 +73,8 @@ function getCredentialWithIcon(name: string | null): ICredentialType | null {
if (type.extends) {
let parentCred = null;
type.extends.forEach((iconName) => {
parentCred = getCredentialWithIcon(iconName);
type.extends.forEach((credType) => {
parentCred = getCredentialWithIcon(credType);
if (parentCred !== null) return;
});
return parentCred;
Expand All @@ -76,23 +85,18 @@ function getCredentialWithIcon(name: string | null): ICredentialType | null {
</script>

<template>
<div>
<img v-if="filePath" :class="$style.credIcon" :src="filePath" />
<NodeIcon v-else-if="relevantNode" :node-type="relevantNode" :size="28" />
<span v-else :class="$style.fallback"></span>
</div>
<N8nNodeIcon
:class="$style.icon"
:type="iconType"
:size="26"
:src="iconSource"
:name="iconName"
:color="iconColor"
/>
</template>

<style lang="scss" module>
.credIcon {
height: 26px;
}
.fallback {
height: 28px;
width: 28px;
display: flex;
border-radius: 50%;
background-color: var(--color-foreground-base);
.icon {
--node-icon-color: var(--color-foreground-dark);
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ function getNodeTypeBase(nodeTypeDescription: INodeTypeDescription, label?: stri
categories: [category],
},
iconUrl: nodeTypeDescription.iconUrl,
iconColor: nodeTypeDescription.iconColor,
outputs: nodeTypeDescription.outputs,
icon: nodeTypeDescription.icon,
defaults: nodeTypeDescription.defaults,
Expand Down
131 changes: 88 additions & 43 deletions packages/editor-ui/src/components/__tests__/CredentialIcon.test.ts
Original file line number Diff line number Diff line change
@@ -1,66 +1,111 @@
import { createTestingPinia } from '@pinia/testing';
import { createTestingPinia, type TestingPinia } from '@pinia/testing';
import type { ICredentialType, INodeTypeDescription } from 'n8n-workflow';
import { mock } from 'vitest-mock-extended';
import type { INodeTypeDescription } from 'n8n-workflow';

import CredentialIcon from '@/components/CredentialIcon.vue';
import { STORES } from '@/constants';
import { groupNodeTypesByNameAndType } from '@/utils/nodeTypes/nodeTypeTransforms';

import { createComponentRenderer } from '@/__tests__/render';
import { useCredentialsStore } from '@/stores/credentials.store';
import { useRootStore } from '@/stores/root.store';
import { useNodeTypesStore } from '../../stores/nodeTypes.store';

const twitterV1 = mock<INodeTypeDescription>({
version: 1,
credentials: [{ name: 'twitterOAuth1Api', required: true }],
iconUrl: 'icons/n8n-nodes-base/dist/nodes/Twitter/x.svg',
});
describe('CredentialIcon', () => {
const renderComponent = createComponentRenderer(CredentialIcon, {
pinia: createTestingPinia(),
global: {
stubs: ['n8n-tooltip'],
},
});
let pinia: TestingPinia;

const twitterV2 = mock<INodeTypeDescription>({
version: 2,
credentials: [{ name: 'twitterOAuth2Api', required: true }],
iconUrl: 'icons/n8n-nodes-base/dist/nodes/Twitter/x.svg',
});
beforeEach(() => {
pinia = createTestingPinia({ stubActions: false });
});

const nodeTypes = groupNodeTypesByNameAndType([twitterV1, twitterV2]);
const initialState = {
[STORES.CREDENTIALS]: {},
[STORES.NODE_TYPES]: { nodeTypes },
};

const renderComponent = createComponentRenderer(CredentialIcon, {
pinia: createTestingPinia({ initialState }),
global: {
stubs: ['n8n-tooltip'],
},
});
it('shows correct icon when iconUrl is set on credential', () => {
const testIconUrl = 'icons/n8n-nodes-base/dist/nodes/Test/test.svg';
useCredentialsStore().setCredentialTypes([
mock<ICredentialType>({
name: 'test',
iconUrl: testIconUrl,
}),
]);

describe('CredentialIcon', () => {
const findIcon = (baseElement: Element) => baseElement.querySelector('img');
const { getByRole } = renderComponent({
pinia,
props: {
credentialTypeName: 'test',
},
});

it('shows correct icon for credential type that is for the latest node type version', () => {
const { baseElement } = renderComponent({
pinia: createTestingPinia({ initialState }),
expect(getByRole('img')).toHaveAttribute('src', useRootStore().baseUrl + testIconUrl);
});

it('shows correct icon when icon is set on credential', () => {
useCredentialsStore().setCredentialTypes([
mock<ICredentialType>({
name: 'test',
icon: 'fa:clock',
iconColor: 'azure',
}),
]);

const { getByRole } = renderComponent({
pinia,
props: {
credentialTypeName: 'twitterOAuth2Api',
credentialTypeName: 'test',
},
});

expect(findIcon(baseElement)).toHaveAttribute(
'src',
'/icons/n8n-nodes-base/dist/nodes/Twitter/x.svg',
);
const icon = getByRole('img', { hidden: true });
expect(icon.tagName).toBe('svg');
expect(icon).toHaveClass('fa-clock');
});

it('shows correct icon for credential type that is for an older node type version', () => {
it('shows correct icon when credential has an icon with node: prefix', () => {
const testIconUrl = 'icons/n8n-nodes-base/dist/nodes/Test/test.svg';
useCredentialsStore().setCredentialTypes([
mock<ICredentialType>({
name: 'test',
icon: 'node:n8n-nodes-base.test',
iconColor: 'azure',
}),
]);

useNodeTypesStore().setNodeTypes([
mock<INodeTypeDescription>({
version: 1,
name: 'n8n-nodes-base.test',
iconUrl: testIconUrl,
}),
]);

const { getByRole } = renderComponent({
pinia,
props: {
credentialTypeName: 'test',
},
});

expect(getByRole('img')).toHaveAttribute('src', useRootStore().baseUrl + testIconUrl);
});

it('shows fallback icon when icon is not found', () => {
useCredentialsStore().setCredentialTypes([
mock<ICredentialType>({
name: 'test',
icon: 'node:n8n-nodes-base.test',
iconColor: 'azure',
}),
]);

const { baseElement } = renderComponent({
pinia: createTestingPinia({ initialState }),
pinia,
props: {
credentialTypeName: 'twitterOAuth1Api',
credentialTypeName: 'test',
},
});

expect(findIcon(baseElement)).toHaveAttribute(
'src',
'/icons/n8n-nodes-base/dist/nodes/Twitter/x.svg',
);
expect(baseElement.querySelector('.nodeIconPlaceholder')).toBeInTheDocument();
});
});
Loading

0 comments on commit 7073ec6

Please sign in to comment.