fix(core): Stop copying icons, and instead serve them directly from t…

…he source path Fixes #4973 & #5274
n8n-io · Feb 8, 2023 · 0dbac66 · 0dbac66
1 parent f9adbba
commit 0dbac66
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 33 deletions.
diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts
@@ -18,13 +18,7 @@ import type {
 import { LoggerProxy, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
 
 import { createWriteStream } from 'fs';
-import {
-	access as fsAccess,
-	copyFile,
-	mkdir,
-	readdir as fsReaddir,
-	stat as fsStat,
-} from 'fs/promises';
+import { access as fsAccess, mkdir, readdir as fsReaddir, stat as fsStat } from 'fs/promises';
 import path from 'path';
 import config from '@/config';
 import type { InstalledPackages } from '@db/entities/InstalledPackages';
@@ -407,24 +401,6 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 			this.types.nodes = this.types.nodes.concat(types.nodes);
 			this.types.credentials = this.types.credentials.concat(types.credentials);
 
-			// Copy over all icons and set `iconUrl` for the frontend
-			const iconPromises = Object.entries(types).flatMap(([typeName, typesArr]) =>
-				typesArr.map((type) => {
-					if (!type.icon?.startsWith('file:')) return;
-					const icon = type.icon.substring(5);
-					const iconUrl = `icons/${typeName}/${type.name}${path.extname(icon)}`;
-					delete type.icon;
-					type.iconUrl = iconUrl;
-					const source = path.join(dir, icon);
-					const destination = path.join(GENERATED_STATIC_DIR, iconUrl);
-					return mkdir(path.dirname(destination), { recursive: true }).then(async () =>
-						copyFile(source, destination),
-					);
-				}),
-			);
-
-			await Promise.all(iconPromises);
-
 			// Nodes and credentials that have been loaded immediately
 			for (const nodeTypeName in loader.nodeTypes) {
 				this.loaded.nodes[nodeTypeName] = loader.nodeTypes[nodeTypeName];

diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts
@@ -1291,6 +1291,19 @@ class Server extends AbstractServer {
 				},
 			};
 
+			for (const [dir, loader] of Object.entries(this.loadNodesAndCredentials.loaders)) {
+				const pathPrefix = `/icons/${loader.packageName}`;
+				this.app.use(`${pathPrefix}/*/*.(svg|png)`, async (req, res) => {
+					const filePath = pathResolve(dir, req.originalUrl.substring(pathPrefix.length + 1));
+					try {
+						await fsAccess(filePath);
+						res.sendFile(filePath);
+					} catch {
+						res.sendStatus(404);
+					}
+				});
+			}
+
 			this.app.use(
 				'/',
 				express.static(GENERATED_STATIC_DIR),

diff --git a/packages/core/src/DirectoryLoader.ts b/packages/core/src/DirectoryLoader.ts
@@ -48,27 +48,29 @@ export abstract class DirectoryLoader {
 		protected readonly includeNodes: string[] = [],
 	) {}
 
+	abstract packageName: string;
 	abstract loadAll(): Promise<void>;
 
 	protected resolvePath(file: string) {
 		return path.resolve(this.directory, file);
 	}
 
-	protected loadNodeFromFile(packageName: string, nodeName: string, filePath: string) {
+	protected loadNodeFromFile(nodeName: string, filePath: string) {
 		let tempNode: INodeType | IVersionedNodeType;
 		let nodeVersion = 1;
+		const isCustom = this.packageName === 'CUSTOM';
 
 		try {
 			tempNode = loadClassInIsolation(filePath, nodeName);
-			this.addCodex({ node: tempNode, filePath, isCustom: packageName === 'CUSTOM' });
+			this.addCodex({ node: tempNode, filePath, isCustom });
 		} catch (error) {
 			Logger.error(
 				`Error loading node "${nodeName}" from: "${filePath}" - ${(error as Error).message}`,
 			);
 			throw error;
 		}
 
-		const fullNodeName = `${packageName}.${tempNode.description.name}`;
+		const fullNodeName = `${this.packageName}.${tempNode.description.name}`;
 
 		if (this.includeNodes.length && !this.includeNodes.includes(fullNodeName)) {
 			return;
@@ -88,12 +90,12 @@ export abstract class DirectoryLoader {
 			}
 
 			const currentVersionNode = tempNode.nodeVersions[tempNode.currentVersion];
-			this.addCodex({ node: currentVersionNode, filePath, isCustom: packageName === 'CUSTOM' });
+			this.addCodex({ node: currentVersionNode, filePath, isCustom });
 			nodeVersion = tempNode.currentVersion;
 
 			if (currentVersionNode.hasOwnProperty('executeSingle')) {
 				Logger.warn(
-					`"executeSingle" will get deprecated soon. Please update the code of node "${packageName}.${nodeName}" to use "execute" instead!`,
+					`"executeSingle" will get deprecated soon. Please update the code of node "${this.packageName}.${nodeName}" to use "execute" instead!`,
 					{ filePath },
 				);
 			}
@@ -236,7 +238,8 @@ export abstract class DirectoryLoader {
 		if (obj.icon?.startsWith('file:')) {
 			const iconPath = path.join(path.dirname(filePath), obj.icon.substring(5));
 			const relativePath = path.relative(this.directory, iconPath);
-			obj.icon = `file:${relativePath}`;
+			obj.iconUrl = `icons/${this.packageName}/${relativePath.substring(5)}`;
+			delete obj.icon;
 		}
 	}
 }
@@ -246,6 +249,8 @@ export abstract class DirectoryLoader {
  * e.g. `~/.n8n/custom`
  */
 export class CustomDirectoryLoader extends DirectoryLoader {
+	packageName = 'CUSTOM';
+
 	override async loadAll() {
 		const filePaths = await glob('**/*.@(node|credentials).js', {
 			cwd: this.directory,
@@ -256,7 +261,7 @@ export class CustomDirectoryLoader extends DirectoryLoader {
 			const [fileName, type] = path.parse(filePath).name.split('.');
 
 			if (type === 'node') {
-				this.loadNodeFromFile('CUSTOM', fileName, filePath);
+				this.loadNodeFromFile(fileName, filePath);
 			} else if (type === 'credentials') {
 				this.loadCredentialFromFile(fileName, filePath);
 			}
@@ -300,7 +305,7 @@ export class PackageDirectoryLoader extends DirectoryLoader {
 				const filePath = this.resolvePath(node);
 				const [nodeName] = path.parse(node).name.split('.');
 
-				this.loadNodeFromFile(this.packageName, nodeName, filePath);
+				this.loadNodeFromFile(nodeName, filePath);
 			}
 		}