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,27 +33,29 @@ 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}'`);
+            /* v8 ignore next */
+            logger.error(`Failed to load external converter '${newName ?? name}'`);
             logger.error(`Check the code for syntax error and make sure it is up to date with the current Zigbee2MQTT version.`);
             logger.error(
                 `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,16 +31,15 @@ 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> {
+    protected async loadJS(name: string, mod: TModule, newName?: string): Promise<void> {
         // stop if already started
-        await this.enableDisableExtension(false, module.name);
+        await this.enableDisableExtension(false, mod.name);
         await this.addExtension(
-            // @ts-expect-error `module` is the interface, not the actual passed class
-            new module(
+            new mod(
                 this.zigbee,
                 this.mqtt,
                 this.state,
@@ -49,11 +48,13 @@ export default class ExternalExtensions extends ExternalJSExtension<ModuleExport
                 this.enableDisableExtension,
                 this.restartCallback,
                 this.addExtension,
+                // @ts-expect-error additional params that don't fit the internal `Extension` type
                 settings,
                 logger,
             ),
         );
 
-        logger.info(`Loaded external extension '${name}'.`);
+        /* v8 ignore next */
+        logger.info(`Loaded external extension '${newName ?? name}'.`);
     }
 }

lib/extension/externalJS.ts

@@ -2,7 +2,6 @@ import type {Zigbee2MQTTAPI, Zigbee2MQTTResponse} from 'lib/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';
@@ -64,7 +63,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 +99,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 +114,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 +135,47 @@ 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);
+        }
 
         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));
+            const mod = await import(path.join(this.basePath, extension.name));
+
+            await this.loadJS(extension.name, mod.default);
         }
     }
 
@@ -169,23 +191,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,16 @@
+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');
+    }
+}
+
+export default Example2;

test/assets/external_extensions/mjs/exampleExtension.mjs

@@ -0,0 +1,16 @@
+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');
+    }
+}
+
+export default Example;