Skip to content

v1.63: ESLint in CI and a syntax check of every script - #61

Merged
RickUsingGitHub merged 1 commit into
mainfrom
claude/sleepy-bardeen-z0ss8o
Nov 10, 2026
Merged

RickUsingGitHub merged 1 commit into
mainfrom
claude/sleepy-bardeen-z0ss8o

Conversation

@RickUsingGitHub

Copy link
Copy Markdown
Owner

Step 1 of 2 of the structural clean-up left over from the v1.62 code review. No change in behaviour. Step 2 (splitting js/app.js) follows as its own pull request on top of this one.

What's added

  • eslint.config.js (ESLint 10, flat config) suited to the code as it is:
    • js/*.js are linted as classic browser scripts (no modules), with Chart, pdfjsLib and module as the only extra globals. Amber needs none: each file declares it.
    • tests/unit-*.js, scripts/ and the config files are CommonJS; the Playwright specs get Node and browser globals.
    • ESLint's recommended set, plus no-unused-vars, no-shadow, no-var, prefer-const, eqeqeq, and the house style from @stylistic/eslint-plugin: 4-space indent, single quotes, semicolons.
  • eslint-rules/no-extra-arguments.js, a small rule of the project's own. ESLint has nothing built in for a call that passes more arguments than the function takes, which is what the demandMonthsHtml leftover was. It checks functions, and Amber.* functions, defined in the same file, and leaves alone anything with ...rest, arguments or a spread call.
  • npm run lint, and a lint step in .github/workflows/test.yml (straight after npm ci, so it fails before the browsers are installed).
  • tests/unit-syntax.js, now first in the unit run. It parses every js/*.js (no other unit script loads help.js, app.js and the other DOM files), and checks that index.html loads each of them once, with app.js last. That second check is there for the split in step 2.

What it found, and what was done

Finding Where Fix
Unused amberRates computePlanTotals (js/costs.js) Removed
Unused perKwh precalculateDailySummaries and also channelPeriodBreakdown (js/costs.js) Removed
A second argument the function ignores demandMonthsHtml(…, 'demandDays'), twice (js/app.js) Removed
Shadowed money (four functions had their own, under a file-level one) js/app.js The file-level one was only used by displayResults, so it moved in there. Each function keeps its own formatter, unchanged
Shadowed th, d; an initial null never read js/planner-ui.js The inner th gave the same output as the outer one, so it went; d renamed
Shadowed sum; unused n planYear, simulateBattery (js/planner.js) Renamed to weighted; removed
Unused id js/calendar.js Object.values instead of Object.entries
A raw byte-order mark inside a regex literal js/estimate.js, js/import.js Written as `` (the same character, but an editor can't strip it)
let that never changes (7) several const
Unused helpers nem12, planner, ui specs; a shadowed name in unit-planner.js Removed or renamed. No assertion changed

Rules tuned rather than code restyled

  • The tests keep their own alignment of long assert conditions and tables of cases (indent checks for call arguments and arrays are off under tests/).
  • preserve-caught-error is off: network errors are replaced on purpose by a plain message.
  • One eslint-disable-next-line in js/redispatch.js, on a tally that isn't read again but should stay right if a step is added after it.

Checks

  • npm run lint: clean.
  • npm run test:unit: passes (383 ok lines).
  • Playwright: 66 of 66 pass, the same 66 as on main before the change.
  • npm run build:css leaves css/app.css unchanged.
  • Version bumped to 1.63 in index.html, js/constants.js and tests/unit-time.js; README.md, CLAUDE.md and docs/ updated.

Generated by Claude Code

Adds an ESLint flat config for the code as it is (classic browser scripts on
window.Amber, CommonJS unit checks and specs; 4-space indent, single quotes,
semicolons), an `npm run lint` script, and a lint step in the Tests workflow.
A small local rule, no-extra-arguments, reports calls that pass more
arguments than a function in the same file takes.

Fixes what it found, with no change in behaviour: the unused amberRates in
computePlanTotals and perKwh in precalculateDailySummaries and
channelPeriodBreakdown, demandMonthsHtml's ignored second argument, shadowed
names (money, th, sum, d), an unused n in simulateBattery, a raw byte-order
mark inside two regex literals, and unused helpers in three specs.

tests/unit-syntax.js now starts the unit run: it parses every js/*.js and
checks that index.html loads each of them once, with app.js last.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016net8VqMzbBJe1deXahjg6
@RickUsingGitHub
RickUsingGitHub marked this pull request as ready for review November 10, 2026 20:43
@RickUsingGitHub
RickUsingGitHub merged commit 46538b8 into main Nov 10, 2026
1 check passed
@RickUsingGitHub
RickUsingGitHub deleted the claude/sleepy-bardeen-z0ss8o branch November 10, 2026 20:43
RickUsingGitHub added a commit that referenced this pull request Nov 15, 2026
v1.63: ESLint in CI and a syntax check of every script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants