diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index ed27256..4389c6c 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -24,6 +24,7 @@ jobs:
node-version: 22
cache: npm
- run: npm ci
+ - run: npm run lint
- run: npx playwright install --with-deps chromium
- run: npm test -- --workers=2
- uses: actions/upload-artifact@v4
diff --git a/CLAUDE.md b/CLAUDE.md
index 13dd7c3..16fb326 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -12,7 +12,8 @@ A browser-only tool for Australian electricity bills (Amber Electric vs other pl
```bash
npm install
npm run build:css # after changing any Tailwind class in index.html or js/; commit css/app.css
-npm run test:unit # Node unit checks (fast)
+npm run lint # ESLint: unused or shadowed variables, undefined names, house style
+npm run test:unit # Node unit checks (fast), starting with a syntax check of every js/*.js
npm test # unit checks + Playwright (starts scripts/static-server.js on :4173)
npx playwright test tests/planner.spec.js --workers=2 # one spec
npm run serve # http://127.0.0.1:4173
@@ -40,5 +41,5 @@ If Playwright can't find its browser, point it at an installed Chromium with `la
- `js/constants.js`: `Amber.APP_VERSION`;
- `tests/unit-time.js`: the version assert.
2. Update `README.md` and `docs/` if behaviour or structure changed.
-3. Run `npm run build:css` if classes changed, then `npm test`.
-4. Open a pull request. Merge only after the GitHub "Tests" check passes (GitHub Pages serves `main`).
+3. Run `npm run build:css` if classes changed, then `npm run lint` and `npm test`.
+4. Open a pull request. Merge only after the GitHub "Tests" check passes: it runs the lint and the tests (GitHub Pages serves `main`).
diff --git a/README.md b/README.md
index 069f2f5..8f6f0e5 100644
--- a/README.md
+++ b/README.md
@@ -92,8 +92,9 @@ The solar and battery planner. From the last 12 months of meter readings (Amber'
## For developers
```bash
-npm install # Tailwind CLI and Playwright
+npm install # Tailwind CLI, ESLint and Playwright
npm run build:css # css/input.css -> css/app.css (commit the result)
+npm run lint # ESLint (eslint.config.js)
npm test # unit checks, then Playwright against a local static server
npm run test:unit # unit checks only
npm run serve # http://127.0.0.1:4173
diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md
index 40c96bf..b0572ba 100644
--- a/docs/ARCHITECTURE.md
+++ b/docs/ARCHITECTURE.md
@@ -25,7 +25,9 @@ scripts/update-prices.js Downloads AEMO prices into data/prices (run daily by a
scripts/update-solar.js Downloads PVGIS weather into data/solar (run by hand)
tests/unit-*.js Node unit checks (no framework: assert + console)
tests/*.spec.js Playwright end-to-end tests (Chromium)
-.github/workflows/ test.yml (tests on PRs and main), update-prices.yml (daily price refresh)
+eslint.config.js ESLint settings (npm run lint)
+eslint-rules/ The project's own ESLint rules (no-extra-arguments)
+.github/workflows/ test.yml (lint and tests on PRs and main), update-prices.yml (daily price refresh)
chart.umd.min.js, chartjs-plugin-zoom.min.js, pdf.min.js, pdf.worker.min.js Vendored libraries
```
@@ -218,9 +220,10 @@ Saved JSON is read through `storedJson` / `storedCustomPlans` in `app.js`: a val
## Build, test and CI
- `npm run build:css` rebuilds `css/app.css`. Tailwind scans `index.html` and `js/**/*.js` for class names, and `tailwind.config.js` safelists classes that are only built dynamically. Commit `css/app.css` after changing classes.
-- **Unit checks**: `tests/unit-*.js`, run with plain `node` (`npm run test:unit`). Each prints `ok:` / `FAIL:` lines and exits non-zero on failure. `unit-planner.js` holds the parity checks: the planner's cost equals Compare's, battery energy balances, finance and ratings.
-- **End-to-end**: `tests/*.spec.js` with Playwright (Chromium) against `scripts/static-server.js` on port 4173 (`playwright.config.js`, `reuseExistingServer: true`). Tests mock Amber's API with `page.route('**/api.amber.com.au/**', …)` and build NEM12 files in code. About 54 tests take about 45 s with two workers.
-- **CI**: `.github/workflows/test.yml` runs `npm ci`, installs Chromium and runs `npm test -- --workers=2` on every pull request and on pushes to `main`. Failed runs upload `test-results/`.
+- **Lint**: `npm run lint` runs ESLint 10 with `eslint.config.js` (a flat config). The files in `js/` are checked as classic browser scripts, and the unit checks, scripts and specs as CommonJS. On top of ESLint's recommended set it reports unused variables, shadowed names, `var`, `==` and a `let` that never changes, and holds the house style (4-space indent, single quotes, semicolons, from `@stylistic/eslint-plugin`). One rule is the project's own: `local/no-extra-arguments` (`eslint-rules/no-extra-arguments.js`) reports a call that passes more arguments than the function takes, for functions and `Amber.*` functions defined in the same file. Rules are tuned to the code rather than the code restyled: the tests keep their own alignment of long asserts, and `preserve-caught-error` is off because network errors are replaced on purpose by plain messages.
+- **Unit checks**: `tests/unit-*.js`, run with plain `node` (`npm run test:unit`). Each prints `ok:` / `FAIL:` lines and exits non-zero on failure. `unit-syntax.js` runs first: it parses every `js/*.js` (no other unit script loads the DOM files such as `help.js` and `app.js`) and checks that `index.html` loads each of them once, with `app.js` last. `unit-planner.js` holds the parity checks: the planner's cost equals Compare's, battery energy balances, finance and ratings.
+- **End-to-end**: `tests/*.spec.js` with Playwright (Chromium) against `scripts/static-server.js` on port 4173 (`playwright.config.js`, `reuseExistingServer: true`). Tests mock Amber's API with `page.route('**/api.amber.com.au/**', …)` and build NEM12 files in code. About 66 tests take about a minute with two workers.
+- **CI**: `.github/workflows/test.yml` runs `npm ci`, `npm run lint`, installs Chromium and runs `npm test -- --workers=2` on every pull request and on pushes to `main`. Failed runs upload `test-results/`.
## Releases
@@ -228,4 +231,4 @@ Each change ships as a version:
1. Bump the version in three places: `index.html` (the `
`, the "Version X." line in the header, and every `?v=` query string), `js/constants.js` (`Amber.APP_VERSION`), and `tests/unit-time.js` (the version assert).
2. `npm run build:css` if classes changed; update the README and these docs if behaviour changed.
-3. Run `npm test`, open a pull request, and merge once the Tests check passes. GitHub Pages serves `main`.
+3. Run `npm run lint` and `npm test`, open a pull request, and merge once the Tests check passes. GitHub Pages serves `main`.
diff --git a/docs/FRONTEND.md b/docs/FRONTEND.md
index 3e27753..ff3945a 100644
--- a/docs/FRONTEND.md
+++ b/docs/FRONTEND.md
@@ -127,6 +127,7 @@ The app is written for everyday Australian householders:
```bash
npm run build:css
+npm run lint
npm test # or: npx playwright test tests/planner.spec.js
npm run serve # then open http://127.0.0.1:4173
```
diff --git a/docs/REVIEW.md b/docs/REVIEW.md
index 57e6341..f7f94d1 100644
--- a/docs/REVIEW.md
+++ b/docs/REVIEW.md
@@ -7,11 +7,12 @@ Notes for anyone reviewing this codebase: what it must keep doing, where mistake
```bash
npm install
npm run build:css
-npm test # ~10 unit scripts, then ~54 Playwright tests (~45 s with --workers=2)
+npm run lint # ESLint
+npm test # ~11 unit scripts, then ~66 Playwright tests (~60 s with --workers=2)
npm run serve # http://127.0.0.1:4173
```
-Without an Amber API key, use the "Not with Amber?" path on Start with a NEM12 file. The tests build them in code (`nem12Year()`, `nem12Battery()` in `tests/planner.spec.js`). CI runs the same `npm test` on every pull request (`.github/workflows/test.yml`).
+Without an Amber API key, use the "Not with Amber?" path on Start with a NEM12 file. The tests build them in code (`nem12Year()`, `nem12Battery()` in `tests/planner.spec.js`). CI runs `npm run lint` and the same `npm test` on every pull request (`.github/workflows/test.yml`).
## Invariants: what must stay true
@@ -44,7 +45,7 @@ These are the properties the tests guard and that users rely on. A change that b
- **`app.js` is a monolith.** Tabs, plan form, results, imports, bill check, Free? and settings live in one IIFE sharing a `state` object. Natural seams: the plan form (`createPlanObjectFromForm`, `applyPlanTemplate`, TOU windows), results rendering, imports (CSV, NEM12, local sites), the bill check, the Free? tab and settings.
- **HTML as strings.** Most results are template strings set with `innerHTML`, with table markup repeated (`th`/`td` helpers exist only in `planner-ui.js`). It's safe while everything is escaped, but verbose and easy to make inconsistent.
-- **No linter, formatter or type checking.** There's no ESLint or Prettier config; the style is 4-space indentation, single quotes and semicolons. JSDoc comments exist on many engine functions but not throughout.
+- **No formatter or type checking.** ESLint (`npm run lint`, run in CI) catches unused and shadowed variables, undefined names and calls with too many arguments within a file, and holds the style (4-space indentation, single quotes, semicolons). There's no Prettier, and nothing checks types or calls between files. JSDoc comments exist on many engine functions but not throughout.
- **Home-made unit test harness.** `tests/unit-*.js` use their own `assert` and print `ok:`/`FAIL:`. They work, but give no per-test isolation or coverage.
- **End-to-end tests depend on IDs and wording.** Many assertions match visible text, so copy edits need test edits too.
- **Yearly maintenance is manual.** Each 1 July: the default offers and VDO in `templates.js`, Amber's network tariffs in `networks.js` (FY2026-27 today), Amber's default fixed charges and `AMBER_RATES_CHANGE_DATE` in `constants.js`, rebate and hardware defaults in `PLANNER_FINANCE_DEFAULTS`, and `TEMPLATES_AS_AT`. Market offers drift during the year.
@@ -57,7 +58,7 @@ These are the properties the tests guard and that users rely on. A change that b
1. Is it worth splitting `app.js` along the seams above before a frontend refresh, so the refresh doesn't have to touch pricing code?
2. Should HTML building move to small render helpers (or `` elements in `index.html`) to cut repetition, while staying build-free and within the CSP?
-3. Would a lint/format setup (ESLint + Prettier, run in CI) be welcome, given the no-build constraint?
+3. ESLint now runs in CI. Would a formatter (Prettier) or type checking of the JSDoc (`tsc --checkJs`) be welcome too, given the no-build constraint?
4. Is the planner's battery heuristic good enough, or should it be compared against a simple linear-programming optimum for a few test days to measure how optimistic it is?
5. Should the Solar+? search move to a Web Worker (it runs on the main thread today, yielding with `setTimeout(0)`)? The CSP already allows `worker-src 'self'`.
6. Should the app say explicitly that it's independent of Amber Electric (see FRONTEND.md)?
@@ -67,6 +68,7 @@ These are the properties the tests guard and that users rely on. A change that b
| Test | Covers |
|---|---|
+| `unit-syntax.js` | Every `js/*.js` parses as a classic script; `index.html` loads each one once, `app.js` last |
| `unit-time.js` | Dates, NEM vs local time, TOU windows, app version |
| `unit-bill.js`, `unit-billcheck.js` | Bill reading and line-by-line checks |
| `unit-stats.js` | More Stats |
diff --git a/eslint-rules/no-extra-arguments.js b/eslint-rules/no-extra-arguments.js
new file mode 100644
index 0000000..45e1203
--- /dev/null
+++ b/eslint-rules/no-extra-arguments.js
@@ -0,0 +1,104 @@
+'use strict';
+
+/*
+ * A local ESLint rule: a call that passes more arguments than the function takes. JavaScript
+ * drops the extras silently, so they are usually a leftover from an older signature (or a sign
+ * that the caller expects something the function doesn't do).
+ *
+ * It only looks at calls it can be sure about, inside one file:
+ * - a function declared in that file (`function f(a) {}` or `const f = (a) => {}`), called as `f(…)`;
+ * - a function put on the namespace in that file (`Amber.f = function (a) {}`), called as `Amber.f(…)`.
+ * Functions that take `...rest` or read `arguments` are left alone, and so are calls with `...spread`.
+ */
+
+function isFunction(node) {
+ return !!node && (node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression');
+}
+
+module.exports = {
+ meta: {
+ type: 'problem',
+ docs: { description: 'disallow calling a function with more arguments than it takes' },
+ schema: [{
+ type: 'object',
+ properties: { namespaces: { type: 'array', items: { type: 'string' } } },
+ additionalProperties: false
+ }],
+ messages: {
+ extra: '\'{{name}}\' takes {{takes}} argument{{s}} but is called with {{given}}.'
+ }
+ },
+ create(context) {
+ const sourceCode = context.sourceCode;
+ const namespaces = new Set((context.options[0] && context.options[0].namespaces) || []);
+ const members = new Map(); // 'Amber.name' -> function node, or null when assigned twice
+ const memberCalls = [];
+
+ /** How many arguments the function takes, or null when it takes any number. */
+ function arity(fn) {
+ if (fn.params.some((p) => p.type === 'RestElement')) return null;
+ if (fn.type !== 'ArrowFunctionExpression') {
+ const scope = sourceCode.scopeManager.acquire(fn);
+ const args = scope && scope.set.get('arguments');
+ if (args && args.references.length) return null;
+ }
+ return fn.params.length;
+ }
+
+ /** The one function a name always refers to, or null when that can't be known. */
+ function localFunction(node) {
+ let scope = sourceCode.getScope(node);
+ let variable = null;
+ while (scope && !variable) {
+ variable = scope.set.get(node.name) || null;
+ scope = scope.upper;
+ }
+ if (!variable || variable.defs.length !== 1) return null;
+ const def = variable.defs[0];
+ if (variable.references.some((ref) => ref.isWrite() && !ref.init)) return null;
+ if (def.type === 'FunctionName' && def.node.type === 'FunctionDeclaration') return def.node;
+ if (def.type === 'Variable' && def.node.id.type === 'Identifier' && isFunction(def.node.init)) return def.node.init;
+ return null;
+ }
+
+ function memberName(node) {
+ if (node.type !== 'MemberExpression' || node.computed) return null;
+ if (node.object.type !== 'Identifier' || !namespaces.has(node.object.name)) return null;
+ return `${node.object.name}.${node.property.name}`;
+ }
+
+ function check(call, fn, name) {
+ if (call.arguments.some((a) => a.type === 'SpreadElement')) return;
+ const takes = arity(fn);
+ if (takes == null || call.arguments.length <= takes) return;
+ context.report({
+ node: call.arguments[takes],
+ messageId: 'extra',
+ data: { name, takes, s: takes === 1 ? '' : 's', given: call.arguments.length }
+ });
+ }
+
+ return {
+ AssignmentExpression(node) {
+ const name = memberName(node.left);
+ if (!name) return;
+ members.set(name, members.has(name) || node.operator !== '=' || !isFunction(node.right) ? null : node.right);
+ },
+ CallExpression(node) {
+ if (node.callee.type === 'Identifier') {
+ const fn = localFunction(node.callee);
+ if (fn) check(node, fn, node.callee.name);
+ return;
+ }
+ const name = memberName(node.callee);
+ if (name) memberCalls.push({ node, name });
+ },
+ 'Program:exit'() {
+ memberCalls.forEach(({ node, name }) => {
+ const fn = members.get(name);
+ if (fn) check(node, fn, name);
+ });
+ }
+ };
+ }
+};
diff --git a/eslint.config.js b/eslint.config.js
new file mode 100644
index 0000000..82a1922
--- /dev/null
+++ b/eslint.config.js
@@ -0,0 +1,83 @@
+'use strict';
+
+/*
+ * ESLint for the code as it is: classic browser scripts (each js/*.js is an IIFE adding to
+ * window.Amber, no modules, no build step), Node unit checks and scripts, and Playwright specs.
+ * It looks for real problems (unused or shadowed variables, unreachable code, undefined names)
+ * and holds the house style: 4-space indent, single quotes, semicolons. Run: npm run lint
+ */
+const js = require('@eslint/js');
+const globals = require('globals');
+const stylistic = require('@stylistic/eslint-plugin');
+const noExtraArguments = require('./eslint-rules/no-extra-arguments.js');
+
+// The project's own rules (eslint-rules/).
+const local = { rules: { 'no-extra-arguments': noExtraArguments } };
+
+const rules = {
+ ...js.configs.recommended.rules,
+ 'no-unused-vars': ['error', { args: 'after-used', caughtErrors: 'none' }],
+ 'no-shadow': ['error', { builtinGlobals: false, hoist: 'functions' }],
+ 'no-unreachable': 'error',
+ 'local/no-extra-arguments': ['error', { namespaces: ['Amber'] }],
+ // An error caught from the network is replaced on purpose by a plain message for the householder.
+ 'preserve-caught-error': 'off',
+ 'no-var': 'error',
+ 'prefer-const': ['error', { destructuring: 'all' }],
+ eqeqeq: ['error', 'always', { null: 'ignore' }],
+ '@stylistic/indent': ['error', 4, { SwitchCase: 1 }],
+ '@stylistic/quotes': ['error', 'single', { avoidEscape: true, allowTemplateLiterals: 'always' }],
+ '@stylistic/semi': ['error', 'always']
+};
+
+module.exports = [
+ {
+ // Vendored libraries, built CSS and data are not ours to lint.
+ ignores: ['node_modules/**', '*.min.js', 'css/**', 'data/**', 'test-results/**', 'playwright-report/**']
+ },
+ {
+ files: ['**/*.js'],
+ plugins: { '@stylistic': stylistic, local },
+ linterOptions: { reportUnusedDisableDirectives: 'error' },
+ rules
+ },
+ {
+ // The app: classic scripts sharing window.Amber. The same files are `require`d by the Node
+ // unit checks, hence `module` and `globalThis`. Chart and pdfjsLib are the vendored libraries.
+ files: ['js/**/*.js'],
+ languageOptions: {
+ ecmaVersion: 2022,
+ sourceType: 'script',
+ globals: {
+ ...globals.browser,
+ Chart: 'readonly',
+ pdfjsLib: 'readonly',
+ module: 'readonly'
+ }
+ }
+ },
+ {
+ files: ['tests/unit-*.js', 'scripts/**/*.js', 'eslint-rules/**/*.js', '*.config.js'],
+ languageOptions: {
+ ecmaVersion: 2022,
+ sourceType: 'commonjs',
+ globals: { ...globals.node }
+ }
+ },
+ {
+ // Playwright specs run in Node, with page.evaluate callbacks that run in the page.
+ files: ['tests/**/*.spec.js'],
+ languageOptions: {
+ ecmaVersion: 2022,
+ sourceType: 'commonjs',
+ globals: { ...globals.node, ...globals.browser, Amber: 'readonly', Chart: 'readonly' }
+ }
+ },
+ {
+ // The tests line up long assert conditions and tables of cases their own way.
+ files: ['tests/**/*.js'],
+ rules: {
+ '@stylistic/indent': ['error', 4, { SwitchCase: 1, ArrayExpression: 'off', CallExpression: { arguments: 'off' } }]
+ }
+ }
+];
diff --git a/index.html b/index.html
index 163ecb6..de111a5 100644
--- a/index.html
+++ b/index.html
@@ -4,9 +4,9 @@
- Amber Compare & Export v1.62
+ Amber Compare & Export v1.63
-
+
Skip to the tabs
@@ -19,7 +19,7 @@
Amber Compare & Export
See what your electricity really costs on Amber, check an Amber bill, and find out whether another plan (or Amber) would be cheaper for you.
-
Free and private: everything runs in this browser and nothing is uploaded. Version 1.62.
+
Free and private: everything runs in this browser and nothing is uploaded. Version 1.63.