kenmirrors/MagicMirror
1
0
Fork 0
mirror of https://github.com/MichMich/MagicMirror.git synced 2026-07-23 15:54:34 -07:00
Code Issues Projects Releases Wiki Activity

refactor(utils): drop ajv dependency (#4142)

Browse Source 
This PR removes `ajv` from module position validation in `js/utils.js`
and replaces it with straightforward JavaScript checks. The old schema
only covered a few basic structure rules, so using `ajv` here felt
heavier than necessary. The updated validation keeps the same expected
behavior for valid configs and unknown-position warnings, but the code
path is now easier to read and easier to reason about when something
goes wrong.

Outcome: same validation behavior, one less dependency, less lines of
production code and better test coverage.
This commit is contained in:
Kristjan ESPERANTO
2026-05-06 19:11:47 +02:00
committed by GitHub
parent c97d0947b6
commit 67911546ed
4 changed files with 126 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
js/utils.js
+39 -54
View File
@@ -8,13 +8,11 @@ const discoveredPositionsJSFilename = "js/positions.js";
const { styleText } = require("node:util");
const Log = require("logger");
const Ajv = require("ajv");
const globals = require("globals");
const { Linter } = require("eslint");
const { getConfigFilePath } = require("#server_functions");
const linter = new Linter({ configType: "flat" });
const ajv = new Ajv();
const requireFromString = (src) => {
const m = new module.constructor();
@@ -227,67 +225,54 @@ const checkConfigFile = (configObject) => {
};
/**
* Validates the modules array in the config object.
* Checks that:
* - `modules` is an array
* - every entry has a `module` property of type string
* - every entry's `position` (if set) is a known region from index.html
*
* @param {string} data - The content of the configuration file to validate.
* Unknown positions produce a warning; structural errors are fatal.
* @param {object} data - The full config object to validate.
*/
const validateModulePositions = (data) => {
Log.info("Checking modules structure configuration ...");
const positionList = getModulePositions();
// Make Ajv schema configuration of modules config
// Only scan "module" and "position"
const schema = {
type: "object",
properties: {
modules: {
type: "array",
items: {
type: "object",
properties: {
module: {
type: "string"
},
position: {
type: "string"
}
},
required: ["module"]
}
}
}
};
// Scan all modules
const validate = ajv.compile(schema);
const valid = validate(data);
if (valid) {
Log.info(styleText("green", "Your modules structure configuration doesn't contain errors :)"));
// Check for unknown positions (warning only, not an error)
if (data.modules) {
for (const [index, module] of data.modules.entries()) {
if (module.position && !positionList.includes(module.position)) {
Log.warn(`Module ${index} ("${module.module}") uses unknown position: "${module.position}"`);
Log.warn(`Known positions are: ${positionList.join(", ")}`);
}
}
}
} else {
const module = validate.errors[0].instancePath.split("/")[2];
const position = validate.errors[0].instancePath.split("/")[3];
let errorMessage = "This module configuration contains errors:";
errorMessage += `\n${JSON.stringify(data.modules[module], null, 2)}`;
if (position) {
errorMessage += `\n${position}: ${validate.errors[0].message}`;
errorMessage += `\n${JSON.stringify(validate.errors[0].params.allowedValues, null, 2).slice(1, -1)}`;
} else {
errorMessage += validate.errors[0].message;
}
Log.error(errorMessage);
// `modules` always exists (defaults.js provides a default array), but guard against it being overridden with a non-array value
if (data.modules !== undefined && !Array.isArray(data.modules)) {
Log.error("This module configuration contains errors:\nmodules must be an array");
process.exit(1);
}
// Validate each module entry
for (const [index, mod] of (data.modules ?? []).entries()) {
// Each module entry must be an object so we can safely inspect its fields
if (mod === null || typeof mod !== "object" || Array.isArray(mod)) {
Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule entry must be an object`);
process.exit(1);
}
// `module` (the module name) is required and must be a string
if (typeof mod.module !== "string") {
Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule: must be a string`);
process.exit(1);
}
// `position` is optional, but must be a string when provided
if (mod.position !== undefined && typeof mod.position !== "string") {
Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nposition: must be a string`);
process.exit(1);
}
// `position` is optional, but when set it must match a known region
if (mod.position && !positionList.includes(mod.position)) {
Log.warn(`Module ${index} ("${mod.module}") uses unknown position: "${mod.position}"`);
Log.warn(`Known positions are: ${positionList.join(", ")}`);
}
}
Log.info(styleText("green", "Your modules structure configuration doesn't contain errors :)"));
};
module.exports = { loadConfig, getModulePositions, moduleHasValidPosition, getAvailableModulePositions, checkConfigFile };
package-lock.json
Generated
+4 -2
View File
@@ -7,13 +7,11 @@
"": {
"name": "magicmirror",
"version": "2.37.0-develop",
"hasInstallScript": true,
"license": "MIT",
"dependencies": {
"@fontsource/roboto": "^5.2.10",
"@fontsource/roboto-condensed": "^5.2.8",
"@fortawesome/fontawesome-free": "^7.2.0",
"ajv": "^8.20.0",
"animate.css": "^4.1.1",
"croner": "^10.0.1",
"eslint": "^10.3.0",
@@ -2970,6 +2968,7 @@
"version": "8.20.0",
"resolved": "https://registry.npmjs.org/ajv/-/ajv-8.20.0.tgz",
"integrity": "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA==",
"dev": true,
"license": "MIT",
"dependencies": {
"fast-deep-equal": "^3.1.3",
@@ -5119,6 +5118,7 @@
"version": "3.1.2",
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz",
"integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==",
"dev": true,
"funding": [
{
"type": "github",
@@ -6332,6 +6332,7 @@
"version": "1.0.0",
"resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz",
"integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==",
"dev": true,
"license": "MIT"
},
"node_modules/json-stable-stringify-without-jsonify": {
@@ -8669,6 +8670,7 @@
"version": "2.0.2",
"resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz",
"integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==",
"dev": true,
"license": "MIT",
"engines": {
"node": ">=0.10.0"
package.json
-1
View File
@@ -89,7 +89,6 @@
"@fontsource/roboto": "^5.2.10",
"@fontsource/roboto-condensed": "^5.2.8",
"@fortawesome/fontawesome-free": "^7.2.0",
"ajv": "^8.20.0",
"animate.css": "^4.1.1",
"croner": "^10.0.1",
"eslint": "^10.3.0",
tests/unit/classes/utils_spec.js
+83
View File
@@ -0,0 +1,83 @@
const fs = require("node:fs");
const Log = require("../../../js/logger");
const { checkConfigFile } = require("../../../js/utils");
const createConfigObject = (modules) => ({
configFilename: "config.js",
configContentFull: "module.exports = { modules: [] };",
fullConf: { modules }
});
const runCheck = (modules) => {
checkConfigFile(createConfigObject(modules));
};
const expectExitForModules = (modules) => {
vi.spyOn(process, "exit").mockImplementation((code) => {
throw new Error(`process.exit:${code}`);
});
expect(() => {
runCheck(modules);
}).toThrow("process.exit:1");
};
describe("utils", () => {
let originalReadFileSync;
beforeEach(() => {
originalReadFileSync = fs.readFileSync;
vi.spyOn(fs, "readFileSync").mockImplementation((fileName, ...args) => {
if (fileName === "index.html") {
return "<div class=\"region top_bar\"></div>\n<div class=\"region lower_third\"></div>";
}
return originalReadFileSync.call(fs, fileName, ...args);
});
vi.spyOn(fs, "writeFileSync").mockImplementation(() => {});
vi.spyOn(Log, "info").mockImplementation(() => {});
vi.spyOn(Log, "warn").mockImplementation(() => {});
vi.spyOn(Log, "error").mockImplementation(() => {});
});
afterEach(() => {
vi.restoreAllMocks();
});
it("accepts valid module entries", () => {
expect(() => {
runCheck([
{ module: "clock", position: "top_bar" },
{ module: "newsfeed" }
]);
}).not.toThrow();
expect(Log.error).not.toHaveBeenCalled();
});
it("exits when modules is not an array", () => {
expectExitForModules("not-an-array");
expect(Log.error).toHaveBeenCalledWith("This module configuration contains errors:\nmodules must be an array");
});
it("exits when module field is missing or not a string", () => {
expectExitForModules([{ module: 123, position: "top_bar" }]);
expect(Log.error).toHaveBeenCalled();
expect(Log.error.mock.calls[0][0]).toContain("module: must be a string");
});
it("warns for unknown positions without exiting", () => {
const exitSpy = vi.spyOn(process, "exit").mockImplementation((code) => {
throw new Error(`process.exit:${code}`);
});
expect(() => {
runCheck([{ module: "clock", position: "made_up_region" }]);
}).not.toThrow();
expect(exitSpy).not.toHaveBeenCalled();
expect(Log.warn).toHaveBeenCalled();
expect(Log.warn.mock.calls[0][0]).toContain("uses unknown position");
});
});