feat: typescript migration and use yarn 4#4179
Draft
pierrejochem wants to merge 28 commits into
Draft
Conversation
Set up the foundation for migrating MagicMirror to strict TypeScript while
keeping the existing dual-runtime architecture intact (browser global scripts +
Node CommonJS), with zero changes to hardcoded .js runtime paths.
Approach: author .ts under src/, compile in place back into js/, defaultmodules/,
serveronly/, clientonly/, translations/ (outputs gitignored). No bundler — browser
files stay TS "script" files (no top-level import/export) so tsc emits global-
defining JS 1:1. Two project configs split the DOM (browser) vs node (server) libs.
- tsconfig.base.json + src/tsconfig.{browser,server}.json + editor tsconfig.json
- src/types/globals.d.ts (ambient window globals; promoted from module-types.ts)
and browser-shims.d.ts for the dual CommonJS export guard
- build/build:watch/clean scripts; pre-build hooks on test*/server/start/config:check
so the suite and runtime always execute fresh compiled output
- eslint: typescript-eslint block for src/**/*.ts mirroring the JS @Stylistic style;
ignore compiled output; vitest esbuild target es2022
- *.ts formatted via eslint (tabs), added to .prettierignore; .ts in editorconfig tab group
- migrate js/deprecated.js -> src/js/deprecated.ts to validate the src->output mapping
Gate: full unit suite green (357 passing; the 1 failure is the pre-existing
macOS-only systeminformation test that asserts a Linux platform).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate the leaf server/CommonJS modules and the dual-world defaultmodules/utils
to strict .ts under src/, compiled in place. Runtime behavior unchanged.
- js/utils, js/http_fetcher, js/server_functions, defaultmodules/utils -> src/*.ts
- server_functions now uses ES named exports (tsc emits exports.X under commonjs,
so require("#server_functions") destructuring keeps working) so migrated callers
can `import { getUserAgent } from "#server_functions"` with real resolution
- type internal aliases via ambient declares: src/types/aliases.d.ts (logger,
node_helper -> any until migrated) and src/types/node-globals.d.ts (config,
root_path, version, ...). #-subpath imports map to migrated src via tsconfig paths
- allowJs:false so tsc never pulls/overwrites hand-written .js inputs
- keep require()/module guards for untyped deps and dynamic requires; import typed
packages (node builtins, undici) for real types; fix strict-mode issues
(catch unknown, possibly-undefined env vars, non-null match, class field decls)
- eslint: ignore the 4 new compiled outputs; add src/**/*.d.ts override (declare var)
Gate: build clean, eslint/prettier clean, 357 unit tests pass (the 1 failure is the
pre-existing macOS-only systeminformation test).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate 16 files via parallel translation, runtime behavior unchanged: - 11 weather providers + weather/provider-utils + calendar/calendarfetcherutils -> server .ts (require()/export = kept; typed class fields; strict fixes) - weatherobject, weatherutils, calendarutils -> browser dual-world .ts: kept as global script files (no top-level import/export) with the `if (typeof module !== "undefined") module.exports = X` guard intact, so both <script> loading and the Node test suite keep working - add SunCalc vendor global to globals.d.ts; WeatherObject/WeatherUtils/CalendarUtils remain cross-file script globals (declared by their own .ts) - ignore compiled outputs (incl. defaultmodules/weather/providers/*.js) in git + eslint Gate: build clean, eslint/prettier clean, 357 unit tests pass (the 1 failure is the pre-existing macOS-only systeminformation test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e 3)
Migrate the 8 server-side helper files via parallel translation, behavior unchanged:
- calendar/{node_helper,calendarfetcher}, newsfeed/{node_helper,newsfeedfetcher},
weather/node_helper, updatenotification/{node_helper,git_helper,update_helper} -> src/*.ts
- node_helpers use `import NodeHelper = require("node_helper")` so the upgraded
ambient node_helper module (ThisType<NodeHelperInstance> on create()) types
`this.sendSocketNotification`/`this.name`/custom fields inside the definition
- dynamic provider/module require() paths kept as runtime function-call requires
- require()/export = elsewhere; typed fields, strict fixes
- ignore the 8 compiled outputs in git + eslint
Gate: build/eslint/prettier clean, 357 unit tests pass (1 pre-existing macOS-only failure).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate the global-script browser leaves to .ts, kept as TS *script* files (no
top-level import/export) so tsc emits global-defining JS 1:1. Runtime unchanged.
- js/{class,logger,defaults,animateCSS,socketclient,translator},
translations/translations, defaultmodules/defaultmodules -> src/*.ts
- browser tsconfig sets alwaysStrict:false so no "use strict" is injected — js/class.js
relies on sloppy mode (this===window, arguments.callee). Type-checking stays strict.
- browser-shims.d.ts: module/process/require shims for the dual-world files
(logger/defaults are <script>-loaded AND require()-d in Node)
- globals.d.ts: drop globals now defined by their own migrated source
(cloneObject/Translator/MMSocket/AnimateCSSIn/Out/defaultModules); add Class/io/mmPort
- class.ts keeps the John Resig pattern verbatim (arguments/.apply/xyz probe) with a
scoped eslint-disable; constructor fns get `this: any`
- ignore the 8 compiled outputs in git + eslint
Gate: build/eslint/prettier clean, 357 unit tests pass incl. the JSDOM specs that
load compiled js/class.js and js/translator.js (validates global-script emit).
The 1 failure is the pre-existing macOS-only systeminformation test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate the two browser-core files, kept as TS script files (no top-level import/export) so tsc emits global-defining JS 1:1. Runtime unchanged. - js/module.ts: the Module base + Module.register/create/definitions contract that forever-JS community modules depend on. Typed `const Module: ModuleConstructor = Class.extend(...)` so all consumers (loader, default modules) get a register() whose definition object is ThisType<any> (this.config/this.translate/... resolve). configMerge keeps its variadic `arguments` (scoped eslint-disable); new MMSocket cast. - js/loader.ts: runtime <script>/<link> injection loader (the dynamic module loader). DOM element vars typed (HTMLScriptElement/HTMLLinkElement); explicit default return. - globals.d.ts: drop Module/Loader ambient consts (now defined by their own source); give Class.extend + Module.register ThisType<any>; add Log.debug. - ignore compiled js/module.js + js/loader.js in git + eslint Gate: build/eslint/prettier clean, 357 unit tests pass incl. the JSDOM module specs (cmp_versions, module_spec) that load compiled js/module.js. 1 pre-existing macOS failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tage 6)
Migrate the 8 default-module entry scripts + alert's notificationFx helper via
parallel translation, kept as TS script files (no import/export) so each still
calls window.Module.register(...) on load exactly as before. Runtime unchanged.
- alert/{alert,notificationFx}, calendar/calendar, clock/clock,
compliments/compliments, helloworld/helloworld, newsfeed/newsfeed,
updatenotification/updatenotification, weather/weather -> src/*.ts
- loosen ModuleProperties to its index signature (modules override base methods with
varying signatures); register() keeps ThisType<any> so this.config/translate resolve
- add NotificationFx + Cron ambient globals
- strict fixes preserving behavior: DOM coercions (colSpan number, opacity String()),
Date arithmetic via getTime(), (window as any) theme hooks, (Object as any).groupBy
Gate: build/eslint/prettier clean, 357 unit tests pass (incl. compliments/weather/
calendar default-module specs). 1 pre-existing macOS-only failure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate the final 15 source files; all 63 source files are now TypeScript.
- browser: js/main (window MM orchestrator), js/vendor -> src/*.ts (script files)
- server: js/{app,server,electron,node_helper,releasenotes,ip_access_control,
check_config,systeminformation,alias-resolver}, clientonly/index,
serveronly/{index,watcher}, defaultmodules/calendar/debug -> src/*.ts
- alias-resolver keeps the Module._resolveFilename monkey-patch (private members
accessed via the untyped require result); side-effect server files use `export {}`
so they are modules (avoids global-scope collisions between script files)
- node_helper base: `this: any` on the Class.extend definition methods
- main: `const MM: any`, instanceof (Module as any), config -> `declare var` (reassigned),
Log.setLogLevel added to LogType, optional sendNotification params
- dynamic require() of community modules + hardcoded "js/electron.js" spawn args kept
- globals.d.ts: drop MM/vendor (now self-defined), add modulePositions; collapse the
per-file compiled-output ignore lists into directory globs (all sources live in src/)
Gate: build/eslint/prettier clean, 357 unit tests pass. 1 pre-existing macOS failure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All 63 source files are TypeScript, compiled with `strict: true` and `allowJs: false` (zero .js remaining in src/). Final polish: - add `prepack` build hook so `npm pack` ships compiled JS (+ source maps) - add src/README.md documenting the src->output layout, the two-project (browser/server) no-bundler build, the editing rules (no top-level import/export in browser globals), and an upstream-merge playbook for this fork Strictness: the canonical `strict` bundle is fully enabled and enforced. The extra opt-in flags noUncheckedIndexedAccess and exactOptionalPropertyTypes are intentionally deferred (~54 mostly-mechanical guards, low safety gain over the current any-tolerant cross-module boundaries) — documented as future tightening. Verification: build + eslint + prettier + markdownlint + cspell clean; 357 unit tests pass; 291 server-side e2e assertions pass (server boots from the compiled TS entry points). Remaining failures are environmental only: the macOS-only systeminformation test (asserts a Linux platform) and the browser-render e2e specs (Playwright chromium not installed in this environment). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tighten beyond the strict bundle. The ~54 resulting errors were all unchecked indexed/match access (arr[i], regexMatch[n], split()[n], Object.keys()[0]) now typed `T | undefined`; fixed with non-null assertions where surrounding guards (.length checks, known match shapes, bounded loops) already guarantee presence — no runtime behavior changed. Verification: build clean under the tightened flags; eslint + prettier clean; 357 unit tests pass; full e2e suite 461/461 pass (Playwright chromium installed, so the browser-render specs now run against the compiled TypeScript). Only the macOS-only systeminformation unit test still fails (asserts a Linux platform). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dule.ts
js/module.ts is now a proper `class Module` instead of `Class.extend({...})`. Base
methods/fields are class members with a real `this` type (no more ThisType<any> hack
for the base), and `instanceof Module` now works natively.
The runtime module contract is preserved exactly:
- Module.register / Module.definitions / Module.create unchanged in behavior.
- Module.create still clones the registered definition (cloneObject) and builds an
instance via the dynamic Module.extend(definition).
- Module.extend now returns `class Subclass extends Module` with the definition's
members applied to the subclass prototype, and still wraps any method that calls
`this._super()` so it invokes the overridden base method — the same inheritance
contract community modules (and the default newsfeed module) rely on.
- `defaults`/`requiresVersion` stay prototype-level (assigned on Module.prototype) so
a module definition can shadow them; per-instance state uses class fields. The base
constructor runs the overridable init().
- Module.register keeps `ModuleProperties & ThisType<any>` so default-module
definitions' `this` still type-checks.
class.js (Class/cloneObject) is retained — still used by Module.create (cloneObject)
and by js/node_helper.ts.
Verification: build clean under strict + noUncheckedIndexedAccess + exactOptionalProperty
Types; eslint + prettier clean; 357 unit tests pass (incl. module create/register,
cmp_versions, and newsfeed `_super`); full e2e 461/461 pass (all modules render in a
real browser).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…de_helper.ts
js/node_helper.ts is now a proper `class NodeHelper` instead of `Class.extend({...})`
— typed methods/fields with a real `this`, and no `require("./class")`.
Runtime contract preserved exactly:
- NodeHelper.create(definition) still returns a CONSTRUCTOR (NodeHelper.extend(def)),
which js/app.js instantiates with `new` per module — unchanged from before.
- NodeHelper.extend returns `class Subclass extends NodeHelper` with the definition's
members applied to the subclass prototype, still wrapping any `this._super()` method
so it calls the overridden base — the inheritance contract community node helpers rely on.
- The base constructor runs the overridable init(); checkFetchStatus/checkFetchError stay
as statics. name/path/expressApp/io are runtime-assigned (declared, not instance fields).
- The `node_helper` alias ambient type (ThisType for default node_helper definitions'
`this`) is unchanged; default node_helpers still type-check.
class.js is no longer required by node_helper (only its cloneObject remains in use, by
Module.create). Verification: build clean under the tightened strict flags; eslint +
prettier clean; 357 unit tests pass; e2e 461/461 pass (server loads every default
node_helper and runs the socket lifecycle).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… system Module and NodeHelper are now real TypeScript classes, so the John Resig "Simple JavaScript Inheritance" Class system in js/class.ts is dead code. Removed it; js/class.ts now contains only the cloneObject helper (still used by Module.create to deep-clone a module definition, and loaded as a browser global via index.html). - drop the now-unused `Class` ambient global from globals.d.ts - update the globals.d.ts cross-file note and src/README.md (the class.js sloppy-mode rationale for alwaysStrict:false is gone; alwaysStrict stays off to preserve the historical sloppy-mode loading of browser scripts) class_spec.js already only exercised cloneObject (incl. the lockStrings logging path), so it stays green. Verification: build clean under strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes; eslint + prettier + cspell clean; 357 unit tests pass; e2e 461/461 pass (translations_spec loads class.js for cloneObject; modules render). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert top-level CommonJS require() to ES import syntax throughout the server-side
TypeScript files, giving real cross-module types (the migration previously left these
as any-typed require()). Runtime behavior preserved.
- node builtins / typed npm packages / internal modules -> `import` (default, named,
or `import * as` as appropriate). export=-style internals use a default import;
named-export internals (#server_functions, ../provider-utils) use named imports.
- ip_access_control.ts switched from `export = { ipAccessControl }` to a named
`export function ipAccessControl`, so consumers import it by name.
- side-effect `require("./alias-resolver")` -> `import "./alias-resolver"` (kept first
so the Module._resolveFilename patch runs before any aliased import resolves).
- alias-resolver.ts: node:path/node:module via import (Module cast to any for the
private _resolveFilename/_mmAliasPatched members).
- add src/types/untyped-modules.d.ts ambient stubs for packages without types
(express, suncalc, feedme, html-to-text) so their imports type-check under strict.
- eslint: ignore the internal alias specifiers (logger, node_helper) in
import-x/no-unresolved (resolved at runtime by alias-resolver, typed via aliases.d.ts).
Deliberately left as runtime require() (cannot/should not be static imports):
- dynamic paths (community node_helpers, weather providers, defaultmodules list,
${root}/js/deprecated, check_config's utils, package.json version, watcher configs);
- dual-world browser scripts with no static TS export (./defaults, ./logger, ./vendor);
- ../package.json (rootDir/outDir layout mismatch); lazy in-function requires (pm2,
clientonly's electron/child_process); logger.ts's node:util (browser script).
Also surfaced/fixed a stale defaultmodules/calendar/debug.ts (dev-only tester) against
the current CalendarFetcher API (8-arg constructor, fetchCalendar()).
Verification: build clean under strict + noUncheckedIndexedAccess +
exactOptionalPropertyTypes; eslint + prettier clean; 357 unit tests pass; e2e 461/461
pass (server boots, alias-resolver-first ordering + import interop verified, all
node_helpers load and modules render).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ules
Reduce the TS lint ignore list from 3 blanket rule-disables to 1.
- @typescript-eslint/no-require-imports: off -> error. Converted the remaining
`import X = require(...)` forms (4 node_helpers, utils' logger) to default imports;
the ~20 genuine require() calls that must stay (dynamic runtime paths, dual-world
browser scripts with no static TS export, lazy/optional deps) now each carry a
justified inline `// eslint-disable-next-line ... -- <reason>` instead of a blanket off.
- @typescript-eslint/no-empty-function: off -> error with allow:["methods","arrowFunctions"]
(no-op override points / default callbacks); the few plain empty function expressions
got an explicit `{ /* no-op */ }` body.
- @typescript-eslint/no-explicit-any stays off, now documented: any is confined to
dynamic external boundaries (config, API payloads, DOM). Reducing it is a separate
per-subsystem domain-typing effort.
New accidental require()/empty functions are now caught by lint. Verification: build
clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes); eslint +
prettier clean; 357 unit tests pass; e2e 461/461 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce a shared ambient MMConfig (+ ModuleConfigEntry) interface in src/types/config.d.ts (global type for both the browser and server projects) and type the global `config` / `global.config` / `window.config` as MMConfig instead of `any`. Well-known fields (address, port, language, cors, modules, logLevel, ...) are typed from js/defaults.js; an index signature keeps the object extensible (users and modules add arbitrary keys), and genuinely dynamic fields (httpHeaders, electronOptions, userAgent) stay any — so this is a safe, no-new-behavior refinement of the prior any. Every config.* / global.config.* access across the codebase is now type-checked; this already caught a latent issue (app.ts passed config.modules[].position, string|undefined, to Utils.moduleHasValidPosition(string) — the param is now widened to match the runtime guard that already handled undefined). Verification: build clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes); eslint + prettier clean; 357 unit tests pass; e2e 461/461 pass. Types-only change, no runtime emit difference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ig / WeatherError) Add shared ambient weather types in src/types/weather.d.ts (WeatherDataPoint, WeatherConfig, WeatherError, WeatherDataCallback, WeatherErrorCallback) and apply them across the weather subsystem to replace `any`: - 11 providers: fetcher (HTTPFetcher | null), onDataCallback/onErrorCallback (typed callbacks), setCallbacks/constructor params, and the #generate* data-builder methods now return WeatherDataPoint / WeatherDataPoint[]; config typed WeatherConfig on 8 of 11. - weather/node_helper.ts: initWeatherProvider(config: WeatherConfig); provider data/error callbacks typed WeatherDataPoint[] / WeatherError. - weather.ts (browser module): incoming socket weather arrays typed WeatherDataPoint[], createWeatherObject(data: WeatherDataPoint); weatherobject.ts feelsLike(): number. Raw external-API payloads (parsedData/response/JSON, #convertWeatherType, .map callbacks) intentionally stay `any`. 3 providers (openmeteo/smhi/ukmetofficedatahub) keep `config: any` where optional-field access (config.lon.toFixed, config.type list checks, validateCoordinates(config)) made WeatherConfig noisy — escape hatch, no behavior change. Verification: build clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes); eslint + prettier clean; 357 unit tests pass (provider specs); e2e 461/461 pass (weather module renders). Types-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add shared ambient calendar types in src/types/calendar.d.ts (CalendarEvent, CalendarConfig) and apply them to replace `any`: - calendarfetcher.ts: events: CalendarEvent[]; httpFetcher: HTTPFetcher. - calendarfetcherutils.ts: filterEvents(data, config: CalendarConfig): CalendarEvent[]; newEvents: CalendarEvent[] (the pushed event object is a CalendarEvent). - calendar.ts (browser module): event iterations (map/filter/sort/forEach/groupBy), createEventList(): CalendarEvent[], single-event helper params, and broadcastEvents all typed CalendarEvent; one `as CalendarEvent[]` at the raw socket-payload boundary. CalendarEvent fields are optional with an index signature; startDate/endDate stay `any` (treated as both ms-strings and numbers). Raw ICS/moment/DOM/socket-payload values stay `any`. Two escape hatches: shouldEventBeExcluded keeps `config: any` (optional excludedEvents under noUncheckedIndexedAccess), and event.title (optional) uses `as string` at RegExp.test sites to preserve the existing coercion behavior. Verification: build clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes); eslint + prettier clean; 357 unit tests pass (calendar specs); e2e 461/461 pass (calendar renders). Types-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add shared ambient newsfeed types in src/types/newsfeed.d.ts (NewsItem, NewsfeedConfig)
and apply them to replace `any`:
- newsfeedfetcher.ts: items: NewsItem[]; httpFetcher: HTTPFetcher (the parsed item
object pushed in parser.on("item", ...) is a NewsItem).
- newsfeed/node_helper.ts: createFetcher(feed, config: NewsfeedConfig);
broadcastFeeds' accumulator typed Record<string, NewsItem[]>.
- newsfeed.ts (browser module): item iterations (map/sort/forEach/findIndex),
newsItems/updatedItems: NewsItem[], getUrlPrefix(item: NewsItem); one
`feeds[feed] as NewsItem[]` cast at the raw socket-payload boundary.
NewsItem fields are optional with an index signature; pubdate/publishDate stay `any`
(feed-dependent). Raw feed-parser values, socket payloads, moment/DOM stay `any`.
Two callbacks in generateFeed keep `item: any` (they call .toLowerCase()/.slice() on the
optional title/description directly; typing would need runtime guards = behavior change).
Verification: build clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes);
eslint + prettier clean; 357 unit tests pass; e2e 461/461 pass (newsfeed renders).
Types-only change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er/module) Reduce `any` in the DOM core by typing module instances as the real Module class and DOM nodes as DOM types (no runtime change): - main.ts: modules: Module[]; modulesStarted/createDomObjects/updateDom/ updateDomWithContent/moduleNeedsUpdate/updateModuleContent/hideModule/showModule take `module: Module`; sendNotification sender/sendTo: Module | null (runtime passes null); module[m] undefined-guard from noUncheckedIndexedAccess. DOM elements kept as HTMLElement (existing casts/guards). MM stays `any` (orchestrator literal); animation names / payload / options / the selection-method internals stay `any` (escape hatch: they use .withClass/.exceptModule not on the Module class). - loader.ts: moduleObjects: Module[]; Module.create result Module | undefined; bootstrapModule mObj: Module (module-data info object stays any). thisModule.hide() is called arg-less -> `(thisModule as any).hide()` rather than loosen the hide() signature. - module.ts: hasAnimateIn/hasAnimateOut widened boolean -> `string | false` (they hold animation names at runtime); other instance state stays any. Verification: build clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes); eslint + prettier clean; 357 unit tests pass (incl. JSDOM module specs); e2e 461/461 pass. Types-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dules
Give the alert module's NotificationFx a real type instead of `any`:
- globals.d.ts: NotificationFxOptions + NotificationFxInstance interfaces; declare
NotificationFx as `new (options?: NotificationFxOptions) => NotificationFxInstance`.
- alert.ts: alerts map typed `{ [key]: NotificationFxInstance }`; dropped the two
`new (NotificationFx as any)(...)` casts — `new NotificationFx({...})` now type-checks
its options and the `.show()`/`.dismiss()` calls.
- notificationFx.ts: constructor `options: NotificationFxOptions`.
- helloworld.ts: getTemplateData(): object.
The other default modules need no change: clock.ts is already any-free (tsc infers the
createElement DOM types), and the remaining `any` in compliments/updatenotification is at
genuine dynamic boundaries (notification payloads, Cron timestamp, git-diff status) where
`any` is appropriate.
Verification: build clean (strict + noUncheckedIndexedAccess + exactOptionalPropertyTypes);
eslint + prettier clean; 357 unit tests pass; e2e 461/461 pass. Types-only change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ire declaration
The browser-shims `require` ambient (declared for the dual-world logger.js's
require("node:util")) was `(id: string) => any`. In the combined editor view (root
tsconfig loads both browser-shims and @types/node) it shadowed NodeRequire, so
serveronly/watcher.ts's `require.cache[require.resolve(configPath)]` showed TS2339
"Property 'cache'/'resolve' does not exist" — an editor-only false error (the server
project build, which excludes browser-shims, was always clean).
Merge `cache` and `resolve` onto the shim via `declare namespace require`, so the
require.cache/require.resolve usage type-checks wherever the shim is the effective
`require` type (browser project + editor) while the real NodeRequire still applies in
the server build.
Verification: both project builds clean; root/editor tsconfig no longer reports the
errors; eslint clean. Types-only .d.ts change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No genuinely-dead exports/functions/globals remained after the migration. The only
"unused" code was required-but-unused parameters (Express req, electron handler args,
base notificationReceived payload, etc.). Prefixed them with `_` and enabled
noUnusedLocals + noUnusedParameters in tsconfig.base.json so future dead locals/params
are caught at compile time.
Also folds in working-tree tidy-ups: serveronly/index app.start config typed MMConfig
(dropped the now-redundant `export {}` — the `import app` already makes it a module),
socketclient basePath check simplified, browser-shims unused eslint-disable removed.
Restored the `@typescript-eslint/no-explicit-any: off` line in the TS eslint block that
had been dropped from the working tree (it produced 670 errors at the intentional
dynamic-`any` boundaries); reducing those is a separate domain-typing effort.
Verification: build clean (strict + noUnusedLocals/Parameters + noUncheckedIndexedAccess
+ exactOptionalPropertyTypes); eslint + prettier clean; 357 unit pass; e2e 461/461 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reorganize the TypeScript sources from output-mirroring folders
(src/js, src/defaultmodules) into concern-based folders, so a feature's
browser script and its server node_helper live together:
src/js/ -> src/server/ (Node core) + src/client/ (browser core)
src/defaultmodules/ -> src/modules/<feature>/
Each concern now compiles via its own per-concern tsc project that pins a
single rootDir -> outDir mapping and the correct lib/types, replacing the
former browser/server split. The seven projects (server, client,
modules.server, modules.client, serveronly, clientonly, translations) are
built with `tsc -b`; output is byte-for-output-set identical (63 files into
js/, defaultmodules/, serveronly/, clientonly/, translations/). Runtime
filenames are unchanged (they are contractual: index.html <script>,
package.json main, dynamic require).
Cross-project seams that can no longer be statically imported (a project
may not emit a .ts owned by another rootDir) are bridged with ambient
declarations instead:
- browser globals defined in one project, used in another
(Module, defaultModules, translations) -> consumer-only shims
src/types/{module,client}-consumer.d.ts
- #-subpath modules produced by src/server but consumed by src/modules /
src/serveronly (#http_fetcher, #app, #alias-resolver, #server_functions)
-> ambient `declare module`s in src/types/aliases.d.ts
Server-internal uses of #server_functions switched to a relative
./server_functions import so they keep real types.
The root tsconfig.json stays a noEmit editor/LSP convenience config (one
combined program) and excludes the consumer shims (their globals are
provided by the real defining sources there).
src/README.md documents the new layout and the cross-project rules.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate the toolchain from npm to Yarn 4.14.1, pinned via the `packageManager` field and managed by Corepack. - Add `.yarnrc.yml` with `nodeLinker: node-modules` (keeps the flat node_modules layout electron, pm2 and playwright expect; no PnP). - Replace package-lock.json with yarn.lock; .gitignore now ignores package-lock.json / pnpm-lock.yaml and the Yarn runtime artifacts (.yarn/* except patches/plugins/sdks, .pnp.*), and tracks yarn.lock. - Yarn Berry does not run npm-style user `pre*` lifecycle scripts (and neither does `node --run`), so the implicit "build before X" guarantee the pretest/prestart/preserver/preconfig:check hooks provided is now inlined as `node --run build && ...` into config:check, server, start, start:dev and the test:* scripts. The lifecycle events Yarn does honor (postinstall, prepare, prepack) are kept. - install-mm -> `yarn workspaces focus --production` (Yarn-native prod install); install-mm:dev -> `yarn install && yarn playwright install chromium && node --run build`. - Add jsonc-eslint-parser to devDependencies: it is a peer dependency of eslint-plugin-package-json that npm auto-installed but Yarn does not. CI (.github/workflows): drop the `cache: "npm"` hints, add a `corepack enable` step after setup-node (so the pinned Yarn is on PATH across node-version swaps), and switch npm/npx invocations to yarn (playwright install, @electron/rebuild, serialport, electron-rebuild). Docs: CONTRIBUTING, the bug-report issue template and the npm-registry publish steps in Collaboration.md updated to Yarn (`yarn npm publish`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add CLAUDE.md: repo guidance covering the src/->runtime compile-in-place model, the hard runtime invariants (no top-level import/export in browser script files; runtime assets live in defaultmodules/ not src/; community modules stay JS; contractual output filenames), the Yarn 4 gotchas (no pre* hooks -> build inlined; peers not auto-installed), the command set, the verify-before-done gate, and known pre-existing test/spell failures. - README.md: add a "This fork: TypeScript + Yarn" section with the dev quick-start (corepack enable; node --run install-mm:dev / test / server). - src/README.md: fix the now-stale build section that claimed npm pre* hooks run the build — they don't under Yarn; build is inlined into the main scripts. Use yarn / node --run commands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
What do we do about the 1200 other modules? how does typescript support arrays of different/unknown types. Mixed types |
Collaborator
|
This would be a huge change. Please elaborate why you think we should migrate to typescript and yarn. I use both in other projects, but see more down then up sides for this project at the moment. |
Good point and see this more as a draft. I will invest more time into the modules approach. |
Yarn is not necessary, but with a proper typing in place I see more advantages. Just need to address a proper interface for the modules or at least the existing ones are still compatible. |
Author
|
Collaborator
|
Before you invest more work in this, I suggest we first discuss the overall direction and constraints for a potential TypeScript migration. Simplicity is a core project principle for MagicMirror² (see our manifesto). So my main concern is that a mandatory build step and a full TypeScript migration would significantly increase day-to-day complexity. If the main goal is type safety, I do not think we need to migrate the source to TypeScript right away. A possible middle ground could be:
That would improve type safety while keeping the contributor experience approachable. For a migration of this size, it would have been better to open an issue or a minimal PR first and seek alignment on direction and constraints before investing in the implementation. In its current size, this PR is too large to review effectively, but I think we can use it now to catch up on that discussion. One more process question: we currently do not have a formal AI policy, but it looks like AI was heavily involved in this PR. Was this PR mostly AI-generated? Also, adding a CLAUDE.md file should likely be discussed separately. Thanks for the effort and initiative. Note: I'm only one of the core maintainers, maybe others have different perspectives on this. |
Collaborator
|
Ah, and you need to direct the PR to the |
Author
|
Hi Kristjan,
first of all, nice to meet you, I use your project quite a while, I really like it and I felt like to contribute something to the project.
I am coming more from the backend side, mainly Java, Kotlin and C# where typing is really a benefit. I know Typescript is not 100% compareable with this but a way into that safety direction.
You are right, It would be a better approach to have a discussion in advanced. I really like OpenSource and if I like a project and find some time then I am happy to give something back. That was my motivation.
I use AI for review and in some situations for finding a proper way to leave it as compatible as possible with the current approach with the modules.
Kind regards,
Pierre
…________________________________
Von: Kristjan ESPERANTO ***@***.***>
Gesendet: Freitag, 5. Juni 2026 08:40
An: MagicMirrorOrg/MagicMirror ***@***.***>
Cc: Pierre Jochem ***@***.***>; Author ***@***.***>
Betreff: Re: [MagicMirrorOrg/MagicMirror] feat: typescript migration and use yarn 4 (PR #4179)
[https://avatars.githubusercontent.com/u/35647502?s=20&v=4]KristjanESPERANTO left a comment (MagicMirrorOrg/MagicMirror#4179)<#4179 (comment)>
Before you invest more work in this, I suggest we first discuss the overall direction and constraints for a potential TypeScript migration.
Simplicity is a core project principle for MagicMirror² (see our manifesto<https://docs.magicmirror.builders/about/manifesto.html>). So my main concern is that a mandatory build step and a full TypeScript migration would significantly increase day-to-day complexity. If the main goal is type safety, I do not think we need to migrate the source to TypeScript right away.
A possible middle ground could be:
* Keep the runtime source in JavaScript.
* Introduce TypeScript-based type checking for JavaScript as a separate no-emit validation step.
* Evaluate the result incrementally before considering a broader migration.
That would improve type safety while keeping the contributor experience approachable.
For a migration of this size, it would have been better to open an issue or a minimal PR first and seek alignment on direction and constraints before investing in the implementation. In its current size, this PR is too large to review effectively, but I think we can use it now to catch up on that discussion.
One more process question: we currently do not have a formal AI policy, but it looks like AI was involved in this PR. Was this PR mostly AI-generated, or did you use AI as an assistant while leading the implementation? Also, adding a CLAUDE.md file should likely be discussed separately.
Thanks for the effort and initiative.
Note: I'm only one of the core maintainers, maybe others have different perspectives on this.
—
Reply to this email directly, view it on GitHub<#4179?email_source=notifications&email_token=ACGEL6L2AZRLTC6Z42XM5KT46JTOVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRSHA4TAMBUHE32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4628900497>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACGEL6PZS7FFGMCXRP6BXYL46JTOVAVCNFSM6AAAAACZ24JQ7WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DMMRYHEYDANBZG4>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/ACGEL6MHABVWM5MMUZPT3NT46JTOVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRSHA4TAMBUHE32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/ACGEL6MGXYG5VW7VB7DUWHL46JTOVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINRSHA4TAMBUHE32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Just a proposal for switching to typescript in combination with yarn 4