refactor: rewrite Module as an ES6 class (#4151)

This PR rewrites `module.js` to use a native ES6 `class` instead of
`Class.extend()` - the same old inheritance helper that was removed from
`node_helper.js` in #4147.

The normal module API stays the same: modules still use
`Module.register({...})`. Internally, `Module.create()` now creates a
named subclass for each module, copies over a cloned definition, and
only calls `init()` when it is actually a function.

Outcome: one less file in the browser bundle, no more magic `_super()`
wiring, better stack traces, and tests for the module creation path that
did not exist before. `module.js` is also now a plain class with no
external dependencies - an intentional step towards `export default
Module` when browser scripts eventually move to native ES modules.

Since these changes touch the core of how modules are created, I'd
appreciate a close review and any feedback on edge cases I may have
missed.
This commit is contained in:
Kristjan ESPERANTO
2026-05-12 20:01:12 +02:00
committed by GitHub
parent b38c7b7aa2
commit 79ea2633a7
7 changed files with 368 additions and 296 deletions

View File

@@ -148,7 +148,7 @@ Module.register("newsfeed", {
}
return Promise.resolve(wrapper);
}
return this._super();
return Module.prototype.getDom.call(this);
},
//Override fetching of template name

View File

@@ -50,7 +50,6 @@
<script type="text/javascript" src="js/logger.js"></script>
<script type="text/javascript" src="translations/translations.js"></script>
<script type="text/javascript" src="js/translator.js"></script>
<script type="text/javascript" src="js/class.js"></script>
<script type="text/javascript" src="config/basepath.js"></script>
<script type="text/javascript" src="js/module.js"></script>
<script type="text/javascript" src="js/loader.js"></script>

View File

@@ -1,119 +0,0 @@
/* global Class, xyz */
/*
* Simple JavaScript Inheritance
* By John Resig https://johnresig.com/
*
* Inspired by base2 and Prototype
*
* MIT Licensed.
*/
(function () {
let initializing = false;
const fnTest = (/xyz/).test(function () {
return xyz;
})
? /\b_super\b/
: /.*/;
// The base Class implementation (does nothing)
this.Class = function () {};
// Create a new Class that inherits from this class
Class.extend = function (prop) {
let _super = this.prototype;
/*
* Instantiate a base class (but only create the instance,
* don't run the init constructor)
*/
initializing = true;
const prototype = new this();
initializing = false;
// Make a copy of all prototype properties, to prevent reference issues.
for (const p in prototype) {
prototype[p] = cloneObject(prototype[p]);
}
// Copy the properties over onto the new prototype
for (const name in prop) {
// Check if we're overwriting an existing function
prototype[name]
= typeof prop[name] === "function" && typeof _super[name] === "function" && fnTest.test(prop[name])
? (function (name, fn) {
return function () {
const tmp = this._super;
/*
* Add a new ._super() method that is the same method
* but on the super-class
*/
this._super = _super[name];
/*
* The method only need to be bound temporarily, so we
* remove it when we're done executing
*/
const ret = fn.apply(this, arguments);
this._super = tmp;
return ret;
};
}(name, prop[name]))
: prop[name];
}
/**
* The dummy class constructor
*/
function Class () {
// All construction is actually done in the init method
if (!initializing && this.init) {
this.init.apply(this, arguments);
}
}
// Populate our constructed prototype object
Class.prototype = prototype;
// Enforce the constructor to be what we expect
Class.prototype.constructor = Class;
// And make this class extendable
Class.extend = arguments.callee;
return Class;
};
}());
/**
* Define the clone method for later use. Helper Method.
* @param {object} obj Object to be cloned
* @returns {object} the cloned object
*/
function cloneObject (obj) {
if (obj === null || typeof obj !== "object") {
return obj;
}
if (obj.constructor.name === "RegExp") {
return new RegExp(obj);
}
const temp = obj.constructor(); // give temp the original obj's constructor
for (const key in obj) {
temp[key] = cloneObject(obj[key]);
if (key === "lockStrings") {
Log.log(key);
}
}
return temp;
}
/*************** DO NOT EDIT THE LINE BELOW ***************/
if (typeof module !== "undefined") {
module.exports = Class;
}

View File

@@ -1,10 +1,31 @@
/* global Class, cloneObject, Loader, MMSocket, nunjucks */
/* global Loader, MMSocket, nunjucks */
/*
* Module Blueprint.
* @typedef {Object} Module
*/
const Module = Class.extend({
class Module {
/**
* Initializes per-instance mutable state.
*/
constructor () {
// Timer reference used for showHide animation callbacks.
this.showHideTimer = null;
/*
* Array to store lockStrings. These strings are used to lock
* visibility when hiding and showing module.
*/
this.lockStrings = [];
/*
* Storage of the nunjucks Environment.
* This should not be referenced directly.
* Use the nunjucksEnvironment() method to get it.
*/
this._nunjucksEnvironment = null;
}
/**
*********************************************************
@@ -12,40 +33,18 @@ const Module = Class.extend({
*********************************************************
*/
// Set the minimum MagicMirror² module version for this module.
requiresVersion: "2.0.0",
// Module config defaults.
defaults: {},
// Timer reference used for showHide animation callbacks.
showHideTimer: null,
/*
* Array to store lockStrings. These strings are used to lock
* visibility when hiding and showing module.
*/
lockStrings: [],
/*
* Storage of the nunjucks Environment,
* This should not be referenced directly.
* Use the nunjucksEnvironment() to get it.
*/
_nunjucksEnvironment: null,
/**
* Called when the module is instantiated.
*/
init () {
},
}
/**
* Called when the module is started.
*/
start () {
Log.info(`Starting module: ${this.name}`);
},
}
/**
* Returns a list of scripts the module requires to be loaded.
@@ -53,7 +52,7 @@ const Module = Class.extend({
*/
getScripts () {
return [];
},
}
/**
* Returns a list of stylesheets the module requires to be loaded.
@@ -61,7 +60,7 @@ const Module = Class.extend({
*/
getStyles () {
return [];
},
}
/**
* Returns a map of translation files the module requires to be loaded.
@@ -71,7 +70,7 @@ const Module = Class.extend({
*/
getTranslations () {
return false;
},
}
/**
* Generates the dom which needs to be displayed. This method is called by the MagicMirror² core.
@@ -104,7 +103,7 @@ const Module = Class.extend({
resolve(div);
}
});
},
}
/**
* Generates the header string which needs to be displayed if a user has a header configured for this module.
@@ -114,7 +113,7 @@ const Module = Class.extend({
*/
getHeader () {
return this.data.header;
},
}
/**
* Returns the template for the module which is used by the default getDom implementation.
@@ -125,7 +124,7 @@ const Module = Class.extend({
*/
getTemplate () {
return `<div class="normal">${this.name}</div><div class="small dimmed">${this.identifier}</div>`;
},
}
/**
* Returns the data to be used in the template.
@@ -134,7 +133,7 @@ const Module = Class.extend({
*/
getTemplateData () {
return {};
},
}
/**
* Called by the MagicMirror² core when a notification arrives.
@@ -148,7 +147,7 @@ const Module = Class.extend({
} else {
Log.debug(`${this.name} received a system notification: ${notification}`);
}
},
}
/**
* Returns the nunjucks environment for the current module.
@@ -170,7 +169,7 @@ const Module = Class.extend({
});
return this._nunjucksEnvironment;
},
}
/**
* Called when a socket notification arrives.
@@ -179,21 +178,21 @@ const Module = Class.extend({
*/
socketNotificationReceived (notification, payload) {
Log.log(`${this.name} received a socket notification: ${notification} - Payload: ${payload}`);
},
}
/**
* Called when the module is hidden.
*/
suspend () {
Log.log(`${this.name} is suspended.`);
},
}
/**
* Called when the module is shown.
*/
resume () {
Log.log(`${this.name} is resumed.`);
},
}
/**
***********************************************
@@ -214,7 +213,7 @@ const Module = Class.extend({
this.hasAnimateOut = false;
this.setConfig(data.config, data.configDeepMerge);
},
}
/**
* Set the module config and combine it with the module defaults.
@@ -223,7 +222,7 @@ const Module = Class.extend({
*/
setConfig (config, deep) {
this.config = deep ? configMerge({}, this.defaults, config) : Object.assign({}, this.defaults, config);
},
}
/**
* Returns a socket object. If it doesn't exist, it's created.
@@ -240,7 +239,7 @@ const Module = Class.extend({
});
return this._socket;
},
}
/**
* Retrieve the path to a module file.
@@ -249,7 +248,7 @@ const Module = Class.extend({
*/
file (file) {
return `${this.data.path}/${file}`.replace("//", "/");
},
}
/**
* Load all required stylesheets by requesting the MM object to load the files.
@@ -257,7 +256,7 @@ const Module = Class.extend({
*/
loadStyles () {
return this.loadDependencies("getStyles");
},
}
/**
* Load all required scripts by requesting the MM object to load the files.
@@ -265,7 +264,7 @@ const Module = Class.extend({
*/
loadScripts () {
return this.loadDependencies("getScripts");
},
}
/**
* Helper method to load all dependencies.
@@ -287,7 +286,7 @@ const Module = Class.extend({
};
await loadNextDependency();
},
}
/**
* Load all translations.
@@ -316,7 +315,7 @@ const Module = Class.extend({
if (translationFile !== translationsFallbackFile) {
return Translator.load(this, translationsFallbackFile, true);
}
},
}
/**
* Request the translation for a given key with optional variables and default value.
@@ -330,7 +329,7 @@ const Module = Class.extend({
return Translator.translate(this, key, defaultValueOrVariables) || defaultValue || "";
}
return Translator.translate(this, key) || defaultValueOrVariables || "";
},
}
/**
* Request an (animated) update of the module.
@@ -338,7 +337,7 @@ const Module = Class.extend({
*/
updateDom (updateOptions) {
MM.updateDom(this, updateOptions);
},
}
/**
* Send a notification to all modules.
@@ -347,7 +346,7 @@ const Module = Class.extend({
*/
sendNotification (notification, payload) {
MM.sendNotification(notification, payload, this);
},
}
/**
* Send a socket notification to the node helper.
@@ -356,7 +355,7 @@ const Module = Class.extend({
*/
sendSocketNotification (notification, payload) {
this.socket().sendNotification(notification, payload);
},
}
/**
* Hide this module.
@@ -383,7 +382,7 @@ const Module = Class.extend({
},
usedOptions
);
},
}
/**
* Show this module.
@@ -411,7 +410,7 @@ const Module = Class.extend({
usedOptions
);
}
});
}
/**
* Merging MagicMirror² (or other) default/config script by `@bugsounet`
@@ -468,11 +467,22 @@ Module.create = function (name) {
const moduleDefinition = Module.definitions[name];
const clonedDefinition = cloneObject(moduleDefinition);
const className = typeof name === "string" && name.trim() ? name : "AnonymousModule";
// Note that we clone the definition. Otherwise the objects are shared, which gives problems.
const ModuleClass = Module.extend(clonedDefinition);
const SubClass = {
[className]: class extends Module {
constructor () {
super();
Object.assign(this, clonedDefinition);
if (typeof this.init === "function") {
this.init();
}
}
}
}[className];
return new ModuleClass();
return new SubClass();
};
Module.register = function (name, moduleDefinition) {
@@ -512,3 +522,43 @@ function cmpVersions (a, b) {
}
return segmentsA.length - segmentsB.length;
}
/**
* Define the clone method for later use. Helper Method.
* @param {object} obj Object to be cloned
* @returns {object} the cloned object
*/
function cloneObject (obj) {
if (obj === null || typeof obj !== "object") {
return obj;
}
if (Array.isArray(obj)) {
return obj.map((item) => cloneObject(item));
}
const tag = Object.prototype.toString.call(obj);
if (tag === "[object RegExp]") {
return new RegExp(obj);
}
if (tag === "[object Date]") {
return new Date(obj.getTime());
}
const proto = Object.getPrototypeOf(obj);
const isPlainObject = proto === null || Object.getPrototypeOf(proto) === null;
// Avoid calling class constructors without "new". Preserve unknown objects by reference.
if (!isPlainObject) {
return obj;
}
const temp = {};
for (const key of Object.keys(obj)) {
temp[key] = cloneObject(obj[key]);
}
return temp;
}

View File

@@ -59,12 +59,10 @@ describe("translations", () => {
const env = createTranslationTestEnvironment();
const window = env.window;
// Load class.js and module.js content directly for loadTranslations tests
const classJs = fs.readFileSync(path.join(__dirname, "..", "..", "js", "class.js"), "utf-8");
// Load module.js content directly for loadTranslations tests
const moduleJs = fs.readFileSync(path.join(__dirname, "..", "..", "js", "module.js"), "utf-8");
// Execute the scripts in the JSDOM context
window.eval(classJs);
// Execute the script in the JSDOM context
window.eval(moduleJs);
// Additional setup for loadTranslations tests

View File

@@ -1,117 +0,0 @@
const path = require("node:path");
const { JSDOM } = require("jsdom");
describe("File js/class", () => {
describe("Test function cloneObject", () => {
let clone;
let dom;
beforeAll(() => {
return new Promise((done) => {
dom = new JSDOM(
`<script>var Log = {log: () => {}};</script>\
<script src="file://${path.join(__dirname, "..", "..", "..", "js", "class.js")}">`,
{ runScripts: "dangerously", resources: "usable" }
);
dom.window.onload = () => {
const { cloneObject } = dom.window;
clone = cloneObject;
done();
};
});
});
it("should clone object", () => {
const expected = { name: "Rodrigo", web: "https://rodrigoramirez.com", project: "MagicMirror" };
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should clone array", () => {
const expected = [1, null, undefined, "TEST"];
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should clone number", () => {
let expected = 1;
let obj = clone(expected);
expect(obj).toBe(expected);
expected = 1.23;
obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone string", () => {
const expected = "Perfect stranger";
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone regex", () => {
const expected = /.*Magic/;
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should clone undefined", () => {
const expected = undefined;
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone null", () => {
const expected = null;
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone nested object", () => {
const expected = {
name: "fewieden",
link: "https://github.com/fewieden",
versions: ["2.0", "2.1", "2.2"],
answerForAllQuestions: 42,
properties: {
items: [{ foo: "bar" }, { lorem: "ipsum" }],
invalid: undefined,
nothing: null
}
};
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
expect(expected.versions === obj.versions).toBe(false);
expect(expected.properties === obj.properties).toBe(false);
expect(expected.properties.items === obj.properties.items).toBe(false);
expect(expected.properties.items[0] === obj.properties.items[0]).toBe(false);
expect(expected.properties.items[1] === obj.properties.items[1]).toBe(false);
});
describe("Test lockstring code", () => {
let log;
beforeAll(() => {
log = dom.window.Log.log;
dom.window.Log.log = (str) => {
expect(str).toBe("lockStrings");
};
});
afterAll(() => {
dom.window.Log.log = log;
});
it("should clone object and log lockStrings", () => {
const expected = { name: "Module", lockStrings: "stringLock" };
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
});
});
});

View File

@@ -0,0 +1,261 @@
const path = require("node:path");
const { JSDOM } = require("jsdom");
describe("File js/module (cloneObject)", () => {
describe("Test function cloneObject", () => {
let clone;
let Module;
let dom;
beforeAll(() => {
return new Promise((done) => {
dom = new JSDOM(
`<script>var Log = {log: () => {}, info: () => {}, warn: () => {}, error: () => {}, debug: () => {}};</script>\
<script src="file://${path.join(__dirname, "..", "..", "..", "js", "module.js")}">`,
{ runScripts: "dangerously", resources: "usable" }
);
dom.window.onload = () => {
const { cloneObject, Module: LoadedModule } = dom.window;
clone = cloneObject;
Module = LoadedModule;
done();
};
});
});
it("should clone object", () => {
const expected = { name: "Rodrigo", web: "https://rodrigoramirez.com", project: "MagicMirror" };
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should clone array", () => {
const expected = [1, null, undefined, "TEST"];
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should clone number", () => {
let expected = 1;
let obj = clone(expected);
expect(obj).toBe(expected);
expected = 1.23;
obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone string", () => {
const expected = "Perfect stranger";
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone regex", () => {
const expected = /.*Magic/;
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should clone date", () => {
const expected = new Date("2026-05-11T20:00:00.000Z");
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
});
it("should return URL by reference", () => {
const expected = new URL("https://magicmirror.builders/path?q=1");
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should return map by reference", () => {
const mapValue = { nested: [1, 2, 3] };
const expected = new Map([["module", mapValue]]);
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should return set by reference", () => {
const setValue = { nested: true };
const expected = new Set([setValue]);
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should return class instances by reference", () => {
class ModuleDefaults {
constructor () {
this.enabled = true;
}
}
const expected = new ModuleDefaults();
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone undefined", () => {
const expected = undefined;
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone null", () => {
const expected = null;
const obj = clone(expected);
expect(obj).toBe(expected);
});
it("should clone nested object", () => {
const expected = {
name: "fewieden",
link: "https://github.com/fewieden",
versions: ["2.0", "2.1", "2.2"],
answerForAllQuestions: 42,
properties: {
items: [{ foo: "bar" }, { lorem: "ipsum" }],
invalid: undefined,
nothing: null
}
};
const obj = clone(expected);
expect(obj).toEqual(expected);
expect(expected === obj).toBe(false);
expect(expected.versions === obj.versions).toBe(false);
expect(expected.properties === obj.properties).toBe(false);
expect(expected.properties.items === obj.properties.items).toBe(false);
expect(expected.properties.items[0] === obj.properties.items[0]).toBe(false);
expect(expected.properties.items[1] === obj.properties.items[1]).toBe(false);
});
describe("Test Module.create", () => {
let info;
beforeEach(() => {
info = dom.window.Log.info;
});
afterEach(() => {
dom.window.Log.info = info;
Module.definitions = {};
});
it("should create module instance with dynamic class name", () => {
const moduleName = "MMM-TestModule";
Module.register(moduleName, {
defaults: {}
});
const moduleInstance = Module.create(moduleName);
expect(moduleInstance.constructor.name).toBe(moduleName);
});
it("should use fallback class name for empty module name", () => {
const moduleName = "";
Module.register(moduleName, {
defaults: {}
});
const moduleInstance = Module.create(moduleName);
expect(moduleInstance.constructor.name).toBe("AnonymousModule");
});
it("should not throw when init is not a function", () => {
const moduleName = "MMM-TestModuleNoInitFunction";
Module.register(moduleName, {
init: null,
defaults: {}
});
expect(() => Module.create(moduleName)).not.toThrow();
});
it("should support lifecycle super call pattern", () => {
const moduleName = "MMM-TestSuperCall";
let loggedMessage;
dom.window.Log.info = (message) => {
loggedMessage = message;
};
Module.register(moduleName, {
defaults: {},
start () {
this.didStart = true;
Module.prototype.start.call(this);
}
});
const moduleInstance = Module.create(moduleName);
moduleInstance.name = moduleName;
moduleInstance.start();
expect(moduleInstance.didStart).toBe(true);
expect(loggedMessage).toBe(`Starting module: ${moduleName}`);
});
it("should set config when defaults are undefined", () => {
const moduleName = "MMM-TestNoDefaults";
Module.register(moduleName, {});
const moduleInstance = Module.create(moduleName);
moduleInstance.setConfig({ foo: "bar" }, false);
expect(moduleInstance.config).toEqual({ foo: "bar" });
moduleInstance.setConfig({ nested: { value: 1 } }, true);
expect(moduleInstance.config).toEqual({ nested: { value: 1 } });
});
it("should initialize lifecycle fields in setData", () => {
const moduleName = "MMM-TestSetData";
Module.register(moduleName, {
defaults: { fromDefaults: true }
});
const moduleInstance = Module.create(moduleName);
moduleInstance.setData({
name: moduleName,
identifier: "module_1",
config: { fromConfig: true },
configDeepMerge: false
});
expect(moduleInstance.name).toBe(moduleName);
expect(moduleInstance.identifier).toBe("module_1");
expect(moduleInstance.hidden).toBe(false);
expect(moduleInstance.hasAnimateIn).toBe(false);
expect(moduleInstance.hasAnimateOut).toBe(false);
expect(moduleInstance.config).toEqual({ fromDefaults: true, fromConfig: true });
});
it("should not share defaults object across module instances", () => {
const moduleName = "MMM-TestDefaultsIsolation";
Module.register(moduleName, {
defaults: {
nested: { value: 1 },
list: [1]
}
});
const firstModuleInstance = Module.create(moduleName);
const secondModuleInstance = Module.create(moduleName);
firstModuleInstance.defaults.nested.value = 42;
firstModuleInstance.defaults.list.push(2);
expect(secondModuleInstance.defaults).toEqual({
nested: { value: 1 },
list: [1]
});
});
});
});
});