Skip to content

Clear the last lint warnings and make them errors - #45

Merged
mtaanquist merged 1 commit into
stagingfrom
lint-sweep
Aug 9, 2026
Merged

Clear the last lint warnings and make them errors#45
mtaanquist merged 1 commit into
stagingfrom
lint-sweep

Conversation

@mtaanquist

Copy link
Copy Markdown
Owner

Closes #37 — the tidy-up pass, kept until last as agreed.

25 warnings → 0, in both halves, at --max-warnings 0. no-explicit-any and no-empty-object-type are errors again.

The any types

apiCall is generic now, so a caller states what it expects:

setDrinks(await apiCall<Drink[]>(`/drinks/bar/${barId}`));

Switching the default to unknown broke 19 call sites — every one a payload that had never been typed. Two were API shapes with no name at all, so SignedIn and BarQrCode are in shared/types.d.ts and the routes return them with satisfies.

The rest: ["--color-fg-default" as any] became a React.CSSProperties assertion on the object; status as any became an OrderStatus parameter; PastOrders took orders: any[] and customerOrder: any; LoginForm built its body as any so it could add a field conditionally, which is a spread now.

The effect dependencies

Fixed rather than silenced, as the issue asked. This only became possible once apiCall was stable (#30) — before that, listing it would have re-run the effects on every render.

The fetchers in BarSelector, BartenderDashboard, CategoriesTab and DrinkForm are memoised on barId and listed properly, so a stale closure can't hold an old bar id. useSessionManager's clearSession likewise — that one sits in the session-timeout effect, exactly where the issue warned a stale closure would produce "it stopped updating".

ImageCropper was the one needing thought: it watched initialCrop.x/.y while the rule wanted the object, but the caller rebuilds that object every render, so depending on it would reset the crop mid-drag. It destructures the two numbers instead.

Fast refresh

useApp and useLiveUpdates, and the contexts they read, moved to hooks/. Those files export no components, so the providers hot-reload properly and the eslint-disable in LiveUpdatesContext is gone.

Keeping it clean

Both lint scripts run with --max-warnings 0 and CI calls them, so warnings can't quietly accumulate again. CLAUDE.md says any is an error in both halves, and that an unknown shape belongs in shared/types.d.ts.

Checked

70 backend tests, both linters, both typecheckers, both builds — clean.

Ran the built image against a copy of the production database and drove it in a headless browser again, since this touched sign-in, the live connection and the session hook:

  • guest page: 18 sections, 302 cards, Gin filter gives 31, surprise-me and Escape work, every endpoint fetched exactly once
  • bartender: reports, drink form populated correctly, add-drink blank, QR modal renders its image and address
  • no console errors anywhere

Closes #37. Both halves are clean at --max-warnings 0, and the two rules
that were downgraded when linting first ran are errors again.

The `any` types are gone. apiCall is generic, so a caller says what it
expects rather than getting something unchecked: apiCall<Drink[]>(...).
The nineteen places that broke were all payloads nobody had ever typed.
Two of them were API shapes with no name, so SignedIn and BarQrCode are in
shared/types.d.ts and the routes return them with `satisfies`.

The missing effect dependencies are fixed rather than silenced, which
apiCall being stable now makes possible: the fetchers are memoised and
listed, so a stale closure cannot quietly hold an old bar id. ImageCropper
watches the crop's x and y rather than the object around them, which the
caller rebuilds every render.

useApp and useLiveUpdates, and the contexts they read, moved into hooks/.
Those files hold no components, so fast refresh works on the providers
again and the disable comment in LiveUpdatesContext is gone.

Both lint scripts now fail on a warning, and CI runs them, so this does not
quietly build back up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mtaanquist
mtaanquist merged commit 4221543 into staging Aug 9, 2026
4 checks passed
@mtaanquist
mtaanquist deleted the lint-sweep branch August 9, 2026 05:13
mtaanquist added a commit that referenced this pull request Aug 9, 2026
* Add toggle to allow bartenders to skip drink order approval steps (#13)

* Initial plan

* Add skip_approval feature to allow auto-accepting orders

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix TypeScript errors in SettingsTab

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix order cancellation not removing orders from bartender's view (#14)

* Initial plan

* Fix order cancellation WebSocket issue by parsing orderId as integer

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Add alphabetical and category sorting to drinks pages (#21)

* Initial plan

* Add alphabetical and category sorting to drinks pages

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix guest access to recipes through favourites endpoint (#20)

* Initial plan

* Fix guest recipe visibility in favourites endpoint

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Add global recipe toggle to pending orders screen for bartenders (#18)

* Initial plan

* Add recipe toggle to pending orders screen for bartenders

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Change recipe toggle to global show/hide for all orders

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Add image crop/resize/reposition feature for drink images (#19)

* Initial plan

* Add image crop/resize/reposition feature for drinks

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Add README

* Fix an issue with websockets not connecting

* Add WebSocket connection error handling and reconnection logic

* Fix image crop/zoom/position display across all components (#22)

* Initial plan

* Fix image crop/zoom/position display in DrinkForm, MenuTab, and random drink modal

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix image crop/zoom/position in RandomDrinkModal, RecipeView, and PastOrders

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix image cropping display issues across all components (#23)

* Initial plan

* Fix image preview CSS - remove conflicting objectPosition property

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Use proper CSS transform with translate for image cropping

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix ImageCropper to sync state with props on subsequent opens

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Revert to transformOrigin approach - wrapper div method didn't work

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Fix crop display issues and reset crop on new image upload

Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>

* Single-image deployment, migrations at startup, and healthchecks (#33)

Serve API, WebSocket, uploads and frontend from one origin. Migrations run in-process at startup, replacing the one-shot db-init container that made Dockge report the stack as exited. Adds healthchecks, a multi-arch GHCR publish workflow, and PUID/PGID support.

Verified against a copy of the production database: no schema drift, 4 to 6 migrations applied cleanly, 155 drinks / 2 bars / 36 orders intact, all 136 images serving.

* Restrict direct access to the container (#34)

* Restrict direct access to the container

Bind the published port to 127.0.0.1 so only a reverse proxy on the same
host can reach the app. It was previously published on all interfaces,
leaving it reachable from the LAN and bypassable — which matters because
QR code URLs are derived from request headers, so a request that skips
the proxy could set those headers itself.

Scope `trust proxy` accordingly. It was unconditionally `true`, trusting
X-Forwarded-* from any source. The default now trusts only loopback and
private addresses, which covers Caddy on the host reaching the container
over the Docker bridge as well as a containerised Caddy on a shared
network, while ignoring the headers from any public source. TRUST_PROXY
overrides it with any value Express accepts.

Document the shared-external-network setup for when Caddy moves into a
container of its own, as a separate stack rather than a service here.

Closes #29

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Shorten comments and move the details to the README

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Add CLAUDE.md and shorten comments throughout (#35)

Records the conventions settled on so far: writing style, the one-origin
shape and why it is that way, how migrations work, the branch and release
flow, and what must never be committed.

Applies the writing rule to the comments added with the single-image work,
which had grown into paragraphs. The reasoning they carried is preserved in
the commit messages and pull requests where it belongs.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Add a test harness and run it in CI (#36)

* Separate building the app from starting it

server.js opened the database and listened on a port as soon as it was
imported, and every route module reached for one shared database handle.
Nothing could be exercised without binding a real port and writing to the
real database.

createApp() now takes the database and folders it should use and returns
the configured app, listening nowhere. Route modules take what they need
instead of importing it. server.js is left as the bootstrap: open the
database, build the app, listen.

No behaviour change; this is what makes the tests in the next commit
possible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Add test and lint tooling to both halves

Vitest and supertest for the backend, plus an ESLint setup it never had.

The frontend had an ESLint config in the old format while depending on
ESLint 9, which only reads the new one — so linting there has never
actually run. Converted it.

That surfaced a backlog. Five real faults are fixed here; the rest are
`any` types and empty object types that issues #27 and #31 already cover,
left as warnings with a note to turn them back into errors once those land.

Also drops the unused uuid dependency and quietens migration output while
tests run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Fix the faults linting turned up

- CategoriesTab declared EditCategoryFormProps twice, verbatim. TypeScript
  merges identical declarations, so it compiled silently.
- LandingPage declared a variable directly inside two switch cases, where
  it is visible to the other branches.
- RecipeView called onClose with a bare && expression.
- DrinkForm caught an error it never used.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Run lint and tests on every push

One job with a fixed name, so branch protection has something stable to
require rather than names that shift when the steps change.

Documents how to run the checks locally and what the tests are for.

Closes #25

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Replace the live update connection with a one-way stream (#38)

Order updates only ever travelled one way. The browser sent a single
message on connecting, to say which bar it was watching; everything else
was the server telling it something had changed.

The old setup never recovered from a dropped connection in any useful
way. It tried three times, back to back rather than spaced out — the
delay it meant to wait was cancelled by its own retry — and then gave up
until someone pressed a button. A phone that sleeps, a moment of bad wifi
or a restart left that guest silently stuck.

Browsers reconnect a one-way stream by themselves, indefinitely, so that
whole problem goes away along with the code that tried to solve it.

Which bar to watch is now part of the address rather than a message sent
after connecting, and a dead connection cleans itself up, so the
heartbeat bookkeeping is gone too. 420 lines become about 90.

Closes #26

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Clear only this app's saved settings when signing out (#39)

Signing out called localStorage.clear(), which wipes everything the
browser has stored for the whole site, not just the bar's own settings.
Now it removes only keys with the app's prefix — which also catches the
activity timestamp it used to leave behind.

Also shares the activity key rather than repeating the literal in two
places.

The duplicate interface and unused imports this issue also listed were
fixed in #36, when linting first ran. The remaining `any` types are left
for #27 and #37.

Closes #31

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Stop uploaded photos piling up (#40)

Photos were only ever removed when their drink was deleted, so two things
left files behind: changing a drink's photo, and starting to add a drink
and then not saving it — the photo is stored before the drink is.

Replacing a photo now removes the old one, unless another drink is using
the same file. The same check applies on delete, which it did not before.

As a backstop, anything nothing refers to and older than a day is cleared
out on start and once a day after. Recent files are left alone in case
someone is still filling in the form.

Run against a copy of the real photos folder, it picks out exactly the
seven unused files found earlier and leaves all 136 in use untouched.

Closes #32

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Move the server to TypeScript (#41)

The API shapes now live in shared/types.d.ts and are imported by both
halves, so a change on one side that the other does not follow becomes a
build error rather than something a guest finds. It holds types only, so
it disappears when built and neither half has to bundle it.

The routes were largely the same few steps written out repeatedly: a
try/catch that logged and returned 500, a lookup to check something
exists, a hand-rolled list of fields to update. Those are now shared:
route() turns a thrown error into the right reply, HttpError carries the
status, findBar/findDrink do the lookups, buildUpdate assembles partial
updates. Roughly 1,600 lines of routes became 1,100 without losing
anything.

Two things this turned up:

- The orders report built its date range by putting a value straight from
  the address bar into the SQL. It is now checked and passed as a
  parameter.
- The frontend claimed drinks had a boolean in_stock while the API sends
  1 or 0. Typed honestly as Flag rather than quietly changing what is
  sent.

Tests and their helpers are TypeScript too, which caught a live-update
test sending a field the update in question does not carry.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Have the pages use the shared types (#42)

The second half of #27. The pages were writing out their own copy of every
API shape in AppContext, and the copy had drifted. They now come from
shared/types.d.ts through frontend/src/types.ts, so the two sides cannot
disagree without the build saying so.

Three things the compiler found once the real shapes were in place:

Settings lost the auto-accept toggle. Signing in replied with a few loose
fields rather than the bar, so the pages pieced one together and dropped
skip_approval doing it. On the QR code path a complete bar was fetched and
then overwritten with the patched-up one. Sign-in now replies with the whole
bar; so does creating one.

Opening a drink with no recipe threw. The recipe was typed as always being
there, but it is nullable, and the recipe view called .match on it.

Flags were typed as booleans. They arrive as 0 or 1 and only worked by
being truthy, so flipping one produced a boolean the API would not take.

Also adds the analytics shapes, which were never typed at all, and a test
for the sign-in reply.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* Notice when an applied migration has been edited (#43)

Migrations were tracked by filename alone, so editing one that had already run
did nothing at all — the database and the repository drifted apart with no
signal. A sha256 of each file is now recorded alongside the name, and a start
that finds a recorded file changed stops with a message saying which one.

Existing databases have rows with no fingerprint. Rather than leave those
unverifiable for good, the first start stamps them with the files as they
stand. Nothing can be proved about them either way, but from then on an edit
is caught like any other.

Also fixes the recovery path the issue flagged. It treated "duplicate column
name" as "this whole file is already applied", so a migration that both
creates a table and adds a column would skip the create if only the column
existed. 2025-09-10-add-categories.sql is exactly that shape. It now applies
the file a statement at a time, stepping over columns that are already there,
and refuses altogether unless every statement is safe to run again.

runMigrations() takes the folder to read, so tests can use their own.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* Break up the oversized components (#44)

* Break up the guest's page

CustomerInterface was 735 lines covering browsing, ordering, favourites and
the surprise-me flow at once. It is 311 now, with the pieces it was made of
pulled out:

- useGuestMenu holds the fetching, grouping and filtering
- DrinkGrid replaces the same seven-prop grid written out five times
- MenuFilters holds the sidebar and the phone dropdown
- OrderPlacedModal was inline

RandomDrinkModal already existed as a component and was never imported —
the page had its own copy of the same markup. It uses the component now.

getStatusIcon and getStatusColor were written out in both CustomerInterface
and OrdersTab. They live in utils/orderStatus, keyed by status so the
compiler catches a missing one, and OrderStatusCard imports them rather than
taking them as props.

apiCall is memoised. It was rebuilt every render, so anything listing it as
a dependency reloaded constantly — the live-updates connection was being
torn down and reopened on every order that came in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Break up the bartender's pages and the shared state

The rest of #30.

AppContext 344 -> 256. The five saved settings each had their own
set-and-persist wrapper written out; they share a useStoredState hook now,
which also survives a corrupt saved value instead of leaving a blank page.
apiCall moved to utils/api, where being a plain function keeps it stable for
free.

DrinkForm 496 -> 281. Adding and changing a drink sent the same body to two
places; that is one call now. The picture field, which is most of the form,
is its own component.

AnalyticsTab 366 -> 209. Four stat cards, two bar charts and two stat lists
were each written out in full; they are StatCard, RankedBars and StatList.

editingDrink was Drink | {} | null, where {} meant "adding one". It is
"new" now, so the type says what it means and the ten "field in drink"
checks in the form are gone.

The base spirits were listed in both the guest's menu and the drink form.
They come from utils/spirits.

Avg orders per day was totalOrders / 7 -- a made-up divisor. The server
sends a real average, over the report's window, so it says which window.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* Clear the last lint warnings and make them errors (#45)

Closes #37. Both halves are clean at --max-warnings 0, and the two rules
that were downgraded when linting first ran are errors again.

The `any` types are gone. apiCall is generic, so a caller says what it
expects rather than getting something unchecked: apiCall<Drink[]>(...).
The nineteen places that broke were all payloads nobody had ever typed.
Two of them were API shapes with no name, so SignedIn and BarQrCode are in
shared/types.d.ts and the routes return them with `satisfies`.

The missing effect dependencies are fixed rather than silenced, which
apiCall being stable now makes possible: the fetchers are memoised and
listed, so a stale closure cannot quietly hold an old bar id. ImageCropper
watches the crop's x and y rather than the object around them, which the
caller rebuilds every render.

useApp and useLiveUpdates, and the contexts they read, moved into hooks/.
Those files hold no components, so fast refresh works on the providers
again and the disable comment in LiveUpdatesContext is gone.

Both lint scripts now fail on a warning, and CI runs them, so this does not
quietly build back up.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* Add tests for the pages (#47)

The frontend had none. These run against a stand-in browser, so they need
no server and no real browser, and they sit alongside the server's in
tests/ under the same Vitest setup.

37 tests, weighted towards things that have actually gone wrong:

- the menu only lists what is in stock, groups by spirit in the bar's order
  rather than alphabetically, sorts inside each group, and files a drink
  with no category under Uncategorized
- a guest cannot order a second drink while one is on the go
- cancelling asks first, and does nothing if you say no
- an order that is cancelled elsewhere disappears from the list
- the connection notice waits out a blip and only speaks up if it lasts
- saved settings survive a refresh, blanks are removed rather than stored,
  an unreadable value falls back instead of blanking the page, and signing
  out leaves other sites' storage alone
- a drink with no picture or no recipe still opens

Two are regression tests for the apiCall identity bug, and both were
checked by putting the old behaviour back: the menu would load in a loop,
and the live connection would be dropped and reopened on every order.

Test helpers: fakeApi() answers requests and records them, signIn() sets up
a session, FakeEventSource stands in for live updates, and aBar/aDrink/
anOrder make something to work with.

CI runs them, along with a typecheck that now covers the test files too.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* Rename to Barkeep, rewrite the README, and fix the past orders page (#48)

* Fix the past orders page, and rewrite the README

Found by walking the app end to end against a copy of the real data.

The past orders page never loaded anything. It read whatever the menu had
already put in shared state, so opening it from the menu worked, but
arriving at the address directly, or refreshing it, showed an empty list.
It loads its own orders now. Two tests cover it, both of which fail against
the old page.

The README described the app as it was three refactors ago: WebSockets, a
/ws path, a port published on every interface, and migrations tracked by
name alone. It now says what the bar actually does, for guests and for the
host, in plain terms, and documents every setting that exists.

Also removes a dead /ws entry from the dev proxy config. Nothing has spoken
WebSocket since the live updates were replaced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Use a plain port in the example compose file

The published port was 21000, which is one particular host's choice, not a
sensible default for an example. Both sides are 3000 now, with a note that
the first number is the one to change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Rename the project to Barkeep, and rewrite the README as an introduction

The README read as an account of what had been wrong and what we did about
it, which is no use to someone seeing this for the first time. It now opens
with what the thing is, walks through how an evening actually goes, then
splits the features into what guests get and what the host gets. The
history is gone; the reasoning that still matters is stated as a plain fact
rather than as a fix.

Renamed throughout: the image is ghcr.io/mtaanquist/barkeep, the compose
service and container are barkeep, the page title and the landing page say
Barkeep, and the three package names follow.

The one exception is the browser storage prefix, which stays homeBarSystem_.
Changing it would sign everyone out on upgrade and strand their settings,
and nobody ever sees it. Both places say why.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant