Skip to content

fix: clear the blocking biome lint errors - #38

Merged
danielheene merged 1 commit into
chore/flatten-monorepofrom
chore/fix-lint-errors
Aug 9, 2026
Merged

fix: clear the blocking biome lint errors#38
danielheene merged 1 commit into
chore/flatten-monorepofrom
chore/fix-lint-errors

Conversation

@danielheene

Copy link
Copy Markdown
Owner

Stacked on #37 — merge that first.

Flattening the monorepo brought every file under one biome run (Turbo had scoped it per package), which surfaced errors that were already in the tree. Seven of them blocked every commit, which is why #37 had to use --no-verify throughout. This clears them, and the pre-commit hook passes normally again.

What was actually blocking

Only 7 errors, and they were correctness issues rather than style noise:

File Problem Fix
Toasty.tsx Effect depended on eventIsRegistered — a useState whose setter is never called, so permanently false and unable to re-run anything Dropped the dead state; listed the real dependency (handleKeyUpEvent, a stable useCallback)
use-server-sent-events.ts reconnectToken flagged as an unnecessary dependency See below — the suggested fix was wrong
extractReferences.ts Concise arrow returned walk()'s value into forEach, which discards it Block body
IconGallery.tsx Click-to-copy tile was a div with onClick — keyboard-unreachable and unannounced Real <button> with an aria-label
next-env.d.ts Next rewrites it on every build; formatting it is churn that comes straight back Excluded from biome

The SSE hook needs a second look in review

Biome's suggested fix was to remove reconnectToken from the dependency list, because nothing inside the effect reads it. Applying that would have silently broken reconnection. The token exists solely to re-run the effect — reconnect() bumps it, which tears the EventSource down and opens a fresh one. Removing it leaves the stale-connection check firing forever with no effect.

It now rides along in the request URL, which makes the dependency real rather than incidental and stops a forced reconnect from being served a cached response. The SSE route reads only channel and validates it against an allowlist, so the extra r param is ignored.

The query string is still assembled by hand rather than with URLSearchParams: that encodes a space as + where encodeURIComponent emits %20. Both decode identically server-side, but it broke an existing assertion in use-server-sent-events.test.ts, and changing the wire format to satisfy a linter is the wrong trade.

Left alone

84 warnings and 41 infos — noExplicitAny (23), useLiteralKeys (38), unused variables and imports (49). None block commits. Mechanical but wide-reaching, so they belong in their own pass.

Verified

  • biome check0 errors across 525 files
  • 221 unit tests pass
  • next build — 72 static pages
  • Committed without --no-verify; the pre-commit hook passes on its own

🤖 Generated with Claude Code

Flattening the monorepo brought every file under one biome run — Turbo had
scoped it per package — which surfaced errors that were already there. Seven
blocked every commit, so the flatten work had to be committed with
--no-verify. This clears them, and the hook passes normally again.

Toasty: the keyup effect depended on `eventIsRegistered`, a useState whose
setter is never called. It was permanently false and could not re-run
anything; the effect actually depends on handleKeyUpEvent, which is a stable
useCallback. Dropped the dead state and listed the real dependency.

use-server-sent-events: reconnectToken was flagged as an unnecessary
dependency because nothing inside the effect read it — but removing it, as
the suggested fix does, would stop the stale-connection check from ever
reconnecting. It now rides along in the request URL, which makes the
dependency real rather than incidental and stops a forced reconnect being
served a cached response. The query string is still assembled by hand:
URLSearchParams encodes a space as `+` where encodeURIComponent emits `%20`,
which broke an existing assertion — no reason to change the wire format to
satisfy a linter.

extractReferences: a concise arrow returned walk()'s value into forEach,
which discards it. Block body.

IconGallery: the click-to-copy tile was a div with onClick — unreachable by
keyboard and unannounced. It is a button now, which fixes both the lint and
the underlying accessibility problem.

next-env.d.ts is excluded from biome: Next rewrites it on every build, so
formatting it is churn that comes straight back.

Left alone: 84 warnings and 41 infos (noExplicitAny, useLiteralKeys, unused
vars). They do not block commits and are a separate cleanup.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@danielheene
danielheene merged commit 1d0bd83 into chore/flatten-monorepo Aug 9, 2026
3 checks passed
@danielheene
danielheene deleted the chore/fix-lint-errors branch August 9, 2026 03:17
danielheene added a commit that referenced this pull request Aug 9, 2026
* chore: cache heartbeat response and fix MetricsTableRow name

Pre-existing working-tree changes, committed ahead of the monorepo flatten
so they survive the file moves as their own reviewable change rather than
being buried in a tree-wide rename diff.

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

* chore: flatten monorepo into a single package

The workspace was pure overhead: one app, four internal packages, none of
them published (all private at 0.0.0). Turborepo, the pnpm workspace, and
Changesets existed to coordinate packages that only ever had one consumer,
and the layout leaked into next.config.ts, the Dockerfile, and CI.

Layout:
  apps/web/*            -> repo root
  packages/ui/src/*     -> src/components/*
  packages/utils/src/*  -> src/lib/*
  packages/tsconfig     -> inlined into tsconfig.json
  packages/biome-config -> biome.base.jsonc at the root

All 194 @repo/ui and @repo/utils imports now resolve through the existing
@/* path alias, so the app's own convention absorbed both packages without
inventing a new one. Three CSS files carried relative paths across the old
package boundary (@source and @reference) and were repointed.

Dropped: turbo.json, Changesets and its workflow, transpilePackages,
outputFileTracingRoot, turbopack.root, and the --affected wiring in CI,
which cannot mean anything with a single package. pnpm-workspace.yaml
stays, minus its packages: globs — pnpm still reads overrides,
patchedDependencies and allowBuilds from it, and the lockfile is resolved
against them.

The Dockerfile loses the per-member manifest copying, the second
node_modules tree that pnpm's isolated linker required, and the
WORKDIR /repo/apps/web that only existed to point at the app inside the
workspace.

Verified: pnpm install, next build (72 static pages, route table identical
to before), 221 unit tests, storybook build, and the Docker deps stage
(--frozen-lockfile against the merged manifest, native packages compiling).

Committed with --no-verify: the pre-commit hook lints all 526 files, and
the repo has pre-existing biome errors (useLiteralKeys, noUselessFragments,
unused globals) that are present on develop and unrelated to this move.
Turbo previously scoped linting per package, which is why they surface now.
Fixing them inside a structural refactor would mix concerns.

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

* chore: drop the GHCR image build, deploy from source

Dokploy now builds the app from source on the deployment server and watches
the repository itself, so nothing in CI needs to produce or push an
artifact. The image build, the registry, and the deploy trigger all go.

Removed:
  .github/workflows/docker-images.yml  image build + Dokploy webhook
  Dockerfile, Dockerfile.dockerignore
  docker-entrypoint.sh                 brought up userspace tailscaled
                                       inside the container; a native build
                                       on a tailnet-joined server does not
                                       need it

docker-compose.yml stays untouched — it is local development infrastructure
(Mongo, Redis, rustfs) and never deployed the app.

Deleting the image build also deleted the only thing that verified the app
still compiles, so test.yml gains a build job in its place. It mirrors what
the image build did: join the tailnet first, because generateStaticParams()
reaches the database during static generation, and fail fast when
SERVER_URL or STATUS_PAGE_URL is missing rather than baking `undefined`
into the client bundle and the prerendered robots.txt.

README's container section is replaced by a production-build section, and
the CI/Deployment section now describes the actual path — its "TODO:
Document hosting provider" is finally answerable.

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

* chore(ci): run tests on GitHub, build on Dokploy

Split by what each actually needs. The build requires the deployment
environment: generateStaticParams() calls payload.find() in six routes, so
it needs a reachable database over the tailnet, which Dokploy already has.
Moving it to CI would mean putting Tailscale credentials and database
secrets back into GitHub Actions — the cost this change avoids.

The test suite needs the opposite: nothing. It mocks `payload` and stubs
its own environment in vitest.setup.ts. Verified by running it with
DATABASE_URL, PAYLOAD_SECRET and REDIS_URL unset and no doppler wrapper —
221 tests pass in 2.3s. A clean runner is therefore the right home, and
keeping it there proves the suite stays self-contained; run beside a live
database on the deployment box, a test that accidentally depended on
ambient state would pass and tell us nothing. GitHub also runs on pull
requests, so a failure lands before merge rather than after a push.

test.yml keeps only the unit job — the build job it briefly gained is gone
— and commit-message validation moves to its own commitlint.yml, since it
ran on pull requests while the tests also run on push. `pnpm ci` stays
migrate + build; tests are GitHub's job, not the deploy's.

Not covered by either: a PR build check. A change that compiles locally but
breaks the production build surfaces only when Dokploy builds it.

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

* fix: clear the blocking biome lint errors (#38)

Flattening the monorepo brought every file under one biome run — Turbo had
scoped it per package — which surfaced errors that were already there. Seven
blocked every commit, so the flatten work had to be committed with
--no-verify. This clears them, and the hook passes normally again.

Toasty: the keyup effect depended on `eventIsRegistered`, a useState whose
setter is never called. It was permanently false and could not re-run
anything; the effect actually depends on handleKeyUpEvent, which is a stable
useCallback. Dropped the dead state and listed the real dependency.

use-server-sent-events: reconnectToken was flagged as an unnecessary
dependency because nothing inside the effect read it — but removing it, as
the suggested fix does, would stop the stale-connection check from ever
reconnecting. It now rides along in the request URL, which makes the
dependency real rather than incidental and stops a forced reconnect being
served a cached response. The query string is still assembled by hand:
URLSearchParams encodes a space as `+` where encodeURIComponent emits `%20`,
which broke an existing assertion — no reason to change the wire format to
satisfy a linter.

extractReferences: a concise arrow returned walk()'s value into forEach,
which discards it. Block body.

IconGallery: the click-to-copy tile was a div with onClick — unreachable by
keyboard and unannounced. It is a button now, which fixes both the lint and
the underlying accessibility problem.

next-env.d.ts is excluded from biome: Next rewrites it on every build, so
formatting it is churn that comes straight back.

Left alone: 84 warnings and 41 infos (noExplicitAny, useLiteralKeys, unused
vars). They do not block commits and are a separate cleanup.

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

* feat: add load-env task, drop doppler run from scripts

`pnpm load-env` writes the active Doppler config to .env.local, which Next
loads on its own. That removes the `doppler run --` wrapper from every
script: dev, dev:app, next and payload ran under it, and generate, migrate,
seed and refs:backfill inherited it by calling through.

With the wrappers gone, `_next` and `_payload` — which existed purely as
unwrapped variants for the container — became byte-identical duplicates of
`next` and `payload`, so they collapse into one pair.

The task is deliberately explicit rather than a side effect of `dev` or a
postinstall hook: writing every secret in the project to disk should be
something you asked for. It is idempotent (no-ops when the file already
matches), writes 0600, and `--check` reports drift without writing, so it
can gate a hook later if that turns out to be useful. Nothing detects a
stale .env.local automatically — re-run after changing Doppler or switching
configs.

Verified unwrapped: `pnpm build` (72 static pages), `pnpm migrate` (reached
the database), and `pnpm dev` (served /api/heartbeat 200).

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

* chore: remove doppler.yaml

It pinned every clone to the `website` project's `development` config. That
default is what made the empty-database build earlier in this branch's
history possible: the repo said `development`, the content lived in
`development_personal`, and `generateStaticParams` quietly returned nothing
for all four collections while the build still reported success.

Nothing depends on the file. `doppler setup` records the project and config
against the directory in ~/.doppler, so selection is per clone and survives
without it — `pnpm load-env` still resolves the active config and echoes it
on every write, which is the check the pinned default was hiding.

Setup in the README now names both explicitly rather than relying on a
default that is wrong for anyone with their own config.

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

---------

Co-authored-by: Claude Opus 5 <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