feat: Use dynamic import for external JS

lib/extension/externalConverters.ts

@@ -1,13 +1,13 @@
-import type * as zhc from 'zigbee-herdsman-converters';
+import type {ExternalDefinitionWithExtend} from 'zigbee-herdsman-converters';
 
 import {addExternalDefinition, removeExternalDefinitions} from 'zigbee-herdsman-converters';
 
 import logger from '../util/logger';
 import ExternalJSExtension from './externalJS';
 
-type ModuleExports = zhc.ExternalDefinitionWithExtend | zhc.ExternalDefinitionWithExtend[];
+type TModule = ExternalDefinitionWithExtend | ExternalDefinitionWithExtend[];
 
-export default class ExternalConverters extends ExternalJSExtension<ModuleExports> {
+export default class ExternalConverters extends ExternalJSExtension<TModule> {
     constructor(
         zigbee: Zigbee,
         mqtt: MQTT,
@@ -33,29 +33,32 @@ export default class ExternalConverters extends ExternalJSExtension<ModuleExport
     }
 
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    protected async removeJS(name: string, module: ModuleExports): Promise<void> {
+    protected async removeJS(name: string, mod: TModule): Promise<void> {
         removeExternalDefinitions(name);
 
         await this.zigbee.resolveDevicesDefinitions(true);
     }
 
-    protected async loadJS(name: string, module: ModuleExports): Promise<void> {
+    protected async loadJS(name: string, mod: TModule, newName?: string): Promise<void> {
         try {
             removeExternalDefinitions(name);
 
-            const definitions = Array.isArray(module) ? module : [module];
+            const definitions = Array.isArray(mod) ? mod : [mod];
+
             for (const definition of definitions) {
-                definition.externalConverterName = name;
+                definition.externalConverterName = newName ?? name;
 
                 addExternalDefinition(definition);
-                logger.info(`Loaded external converter '${name}'.`);
+                logger.info(`Loaded external converter '${newName ?? name}'.`);
             }
 
             await this.zigbee.resolveDevicesDefinitions(true);
         } catch (error) {
-            logger.error(`Failed to load external converter '${name}'`);
-            logger.error(`Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`);
             logger.error(
+                /* v8 ignore next */
+                `Failed to load external converter '${newName ?? name}'. Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`,
+            );
+            logger.warning(
                 `External converters are not meant for long term usage, but for local testing after which a pull request should be created to add out-of-the-box support for the device`,
             );
 

lib/extension/externalExtensions.ts

@@ -4,9 +4,9 @@ import logger from '../util/logger';
 import * as settings from '../util/settings';
 import ExternalJSExtension from './externalJS';
 
-type ModuleExports = typeof Extension;
+type TModule = new (...args: ConstructorParameters<typeof Extension>) => Extension;
 
-export default class ExternalExtensions extends ExternalJSExtension<ModuleExports> {
+export default class ExternalExtensions extends ExternalJSExtension<TModule> {
     constructor(
         zigbee: Zigbee,
         mqtt: MQTT,
@@ -31,29 +31,40 @@ export default class ExternalExtensions extends ExternalJSExtension<ModuleExport
         );
     }
 
-    protected async removeJS(name: string, module: ModuleExports): Promise<void> {
-        await this.enableDisableExtension(false, module.name);
+    protected async removeJS(name: string, mod: TModule): Promise<void> {
+        await this.enableDisableExtension(false, mod.name);
     }
 
-    protected async loadJS(name: string, module: ModuleExports): Promise<void> {
-        // stop if already started
-        await this.enableDisableExtension(false, module.name);
-        await this.addExtension(
-            // @ts-expect-error `module` is the interface, not the actual passed class
-            new module(
-                this.zigbee,
-                this.mqtt,
-                this.state,
-                this.publishEntityState,
-                this.eventBus,
-                this.enableDisableExtension,
-                this.restartCallback,
-                this.addExtension,
-                settings,
-                logger,
-            ),
-        );
+    protected async loadJS(name: string, mod: TModule, newName?: string): Promise<void> {
+        try {
+            // stop if already started
+            await this.enableDisableExtension(false, mod.name);
+            await this.addExtension(
+                new mod(
+                    this.zigbee,
+                    this.mqtt,
+                    this.state,
+                    this.publishEntityState,
+                    this.eventBus,
+                    this.enableDisableExtension,
+                    this.restartCallback,
+                    this.addExtension,
+                    // @ts-expect-error additional params that don't fit the internal `Extension` type
+                    settings,
+                    logger,
+                ),
+            );
+
+            /* v8 ignore start */
+            logger.info(`Loaded external extension '${newName ?? name}'.`);
+        } catch (error) {
+            logger.error(
+                /* v8 ignore next */
+                `Failed to load external extension '${newName ?? name}'. Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`,
+            );
 
-        logger.info(`Loaded external extension '${name}'.`);
+            throw error;
+        }
+        /* v8 ignore stop */
     }
 }

lib/extension/externalJS.ts

@@ -2,7 +2,6 @@ import type {Zigbee2MQTTAPI, Zigbee2MQTTResponse} from '../types/api';
 
 import fs from 'node:fs';
 import path from 'node:path';
-import {Context, runInNewContext} from 'node:vm';
 
 import bind from 'bind-decorator';
 import stringify from 'json-stable-stringify-without-jsonify';
@@ -14,6 +13,7 @@ import utils from '../util/utils';
 import Extension from './extension';
 
 const SUPPORTED_OPERATIONS = ['save', 'remove'];
+const BACKUP_DIR = 'old';
 
 export default abstract class ExternalJSExtension<M> extends Extension {
     protected mqttTopic: string;
@@ -46,12 +46,12 @@ export default abstract class ExternalJSExtension<M> extends Extension {
         await this.publishExternalJS();
     }
 
-    private getFilePath(name: string, mkBasePath: boolean = false): string {
-        if (mkBasePath && !fs.existsSync(this.basePath)) {
-            fs.mkdirSync(this.basePath, {recursive: true});
+    private getFilePath(name: string, mkBasePath: boolean = false, backup: boolean = false): string {
+        if (mkBasePath && !fs.existsSync(backup ? path.join(this.basePath, BACKUP_DIR) : this.basePath)) {
+            fs.mkdirSync(backup ? path.join(this.basePath, BACKUP_DIR) : this.basePath, {recursive: true});
         }
 
-        return path.join(this.basePath, name);
+        return backup ? path.join(this.basePath, BACKUP_DIR, name) : path.join(this.basePath, name);
     }
 
     protected getFileCode(name: string): string {
@@ -64,7 +64,7 @@ export default abstract class ExternalJSExtension<M> extends Extension {
         }
 
         for (const fileName of fs.readdirSync(this.basePath)) {
-            if (fileName.endsWith('.js')) {
+            if (fileName.endsWith('.js') || fileName.endsWith('.cjs') || fileName.endsWith('.mjs')) {
                 yield {name: fileName, code: this.getFileCode(fileName)};
             }
         }
@@ -100,9 +100,9 @@ export default abstract class ExternalJSExtension<M> extends Extension {
         }
     }
 
-    protected abstract removeJS(name: string, module: M): Promise<void>;
+    protected abstract removeJS(name: string, mod: M): Promise<void>;
 
-    protected abstract loadJS(name: string, module: M): Promise<void>;
+    protected abstract loadJS(name: string, mod: M, newName?: string): Promise<void>;
 
     @bind private async remove(
         message: Zigbee2MQTTAPI['bridge/request/converter/remove'] | Zigbee2MQTTAPI['bridge/request/extension/remove'],
@@ -115,8 +115,9 @@ export default abstract class ExternalJSExtension<M> extends Extension {
         const toBeRemoved = this.getFilePath(name);
 
         if (fs.existsSync(toBeRemoved)) {
-            await this.removeJS(name, this.loadModuleFromText(this.getFileCode(name), name));
+            const mod = await import(toBeRemoved);
 
+            await this.removeJS(name, mod.default);
             fs.rmSync(toBeRemoved, {force: true});
             logger.info(`${name} (${toBeRemoved}) removed.`);
             await this.publishExternalJS();
@@ -135,25 +136,61 @@ export default abstract class ExternalJSExtension<M> extends Extension {
         }
 
         const {name, code} = message;
+        const filePath = this.getFilePath(name, true);
+        let newFilePath = filePath;
+        let newName = name;
+
+        if (fs.existsSync(filePath)) {
+            // if file already exist, version it to bypass node module caching
+            const versionMatch = name.match(/\.(\d+)\.(c|m)?js$/);
+
+            if (versionMatch) {
+                const version = parseInt(versionMatch[1], 10);
+                newName = name.replace(`.${version}.`, `.${version + 1}.`);
+            } else {
+                const ext = path.extname(name);
+                newName = name.replace(ext, `.1${ext}`);
+            }
+
+            newFilePath = this.getFilePath(newName, true);
+
+            // move previous version to backup dir
+            fs.renameSync(filePath, this.getFilePath(name, true, true));
+        }
 
         try {
-            await this.loadJS(name, this.loadModuleFromText(code, name));
+            fs.writeFileSync(newFilePath, code, 'utf8');
 
-            const filePath = this.getFilePath(name, true);
+            const mod = await import(newFilePath);
 
-            fs.writeFileSync(filePath, code, 'utf8');
-            logger.info(`${name} loaded. Contents written to '${filePath}'.`);
+            await this.loadJS(name, mod.default, newName);
+            logger.info(`${newName} loaded. Contents written to '${newFilePath}'.`);
             await this.publishExternalJS();
 
             return utils.getResponse(message, {});
         } catch (error) {
-            return utils.getResponse(message, {}, `${name} contains invalid code: ${(error as Error).message}`);
+            fs.rmSync(newFilePath, {force: true});
+
+            return utils.getResponse(message, {}, `${newName} contains invalid code: ${(error as Error).message}`);
         }
     }
 
     private async loadFiles(): Promise<void> {
         for (const extension of this.getFiles()) {
-            await this.loadJS(extension.name, this.loadModuleFromText(extension.code, extension.name));
+            try {
+                const mod = await import(path.join(this.basePath, extension.name));
+
+                await this.loadJS(extension.name, mod.default);
+            } catch (error) {
+                const destPath = this.getFilePath(extension.name, true, true);
+
+                fs.renameSync(this.getFilePath(extension.name), destPath);
+
+                logger.error(
+                    `Invalid external ${this.mqttTopic} '${extension.name}' was moved to '${destPath}' to prevent interference with Zigbee2MQTT.`,
+                );
+                logger.debug((error as Error).stack!);
+            }
         }
     }
 
@@ -169,23 +206,4 @@ export default abstract class ExternalJSExtension<M> extends Extension {
             true,
         );
     }
-
-    private loadModuleFromText(moduleCode: string, name: string): M {
-        const moduleFakePath = path.join(__dirname, '..', '..', 'data', 'extension', name);
-        const sandbox: Context = {
-            require: require,
-            module: {},
-            console,
-            setTimeout,
-            clearTimeout,
-            setInterval,
-            clearInterval,
-            setImmediate,
-            clearImmediate,
-        };
-
-        runInNewContext(moduleCode, sandbox, moduleFakePath);
-
-        return sandbox.module.exports;
-    }
 }

test/assets/external_converters/mock-external-converter.js → test/assets/external_converters/cjs/mock-external-converter.js

@@ -1,9 +1,11 @@
+const {posix} = require('node:path');
+
 const mockDevice = {
     mock: true,
     zigbeeModel: ['external_converter_device'],
     vendor: 'external',
     model: 'external_converter_device',
-    description: 'external',
+    description: posix.join('external', 'converter'),
     fromZigbee: [],
     toZigbee: [],
     exposes: [],

test/assets/external_converters/mjs/mock-external-converter-multiple.mjs

@@ -0,0 +1,22 @@
+export default [
+    {
+        mock: 1,
+        model: 'external_converters_device_1',
+        zigbeeModel: ['external_converter_device_1'],
+        vendor: 'external_1',
+        description: 'external_1',
+        fromZigbee: [],
+        toZigbee: [],
+        exposes: [],
+    },
+    {
+        mock: 2,
+        model: 'external_converters_device_2',
+        zigbeeModel: ['external_converter_device_2'],
+        vendor: 'external_2',
+        description: 'external_2',
+        fromZigbee: [],
+        toZigbee: [],
+        exposes: [],
+    },
+];

test/assets/external_converters/mjs/mock-external-converter.mjs

@@ -0,0 +1,12 @@
+import {posix} from 'node:path';
+
+export default {
+    mock: true,
+    zigbeeModel: ['external_converter_device'],
+    vendor: 'external',
+    model: 'external_converter_device',
+    description: posix.join('external', 'converter'),
+    fromZigbee: [],
+    toZigbee: [],
+    exposes: [],
+};

test/assets/external_extensions/mjs/example2Extension.mjs

@@ -0,0 +1,14 @@
+export default class Example2 {
+    constructor(zigbee, mqtt, state, publishEntityState, eventBus) {
+        this.mqtt = mqtt;
+        this.mqtt.publish('example2/extension', 'call2 from constructor');
+    }
+
+    start() {
+        this.mqtt.publish('example2/extension', 'call2 from start');
+    }
+
+    stop() {
+        this.mqtt.publish('example/extension', 'call2 from stop');
+    }
+}

test/assets/external_extensions/mjs/exampleExtension.mjs

@@ -0,0 +1,14 @@
+export default class Example {
+    constructor(zigbee, mqtt, state, publishEntityState, eventBus) {
+        this.mqtt = mqtt;
+        this.mqtt.publish('example/extension', 'call from constructor');
+    }
+
+    start() {
+        this.mqtt.publish('example/extension', 'call from start');
+    }
+
+    stop() {
+        this.mqtt.publish('example/extension', 'call from stop');
+    }
+}