From 2710fbcf49047e08805fb965ab9ce9b5b68999db Mon Sep 17 00:00:00 2001 From: Aaron Elijah Mars <61592645+aaronjmars@users.noreply.github.com> Date: Mon, 20 Jul 2026 12:46:57 -0400 Subject: [PATCH] fix: call ColumnCard hooks unconditionally, enforce lint in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ColumnCard returned its "Unknown column type" card before ~10 hooks. A column's type resolves through the plugin registry and can flip between renders, so that early return could change hook order and crash the component. Move the bail-out below every hook; the hooks above already tolerate a missing type (`paginated` optional-chains, the refresh effect guards itself). Three refs were also mutated during render, which is unsafe under concurrent rendering — a render that never commits would still have written. Sync them in effects instead; the useRef seeds cover the first frame and both consumers only dereference asynchronously. The remaining set-state-in-effect reports are suppressed with rationale rather than restructured: sticky search-open cannot be derived without auto-closing mid-type, and the other two sync from outside React. Also fix 6 unescaped entities, drop the unused MODE_LABELS in the producthunt client, and treat a leading underscore as deliberately-unused so plugin ConfigForms can keep documenting the props contract they satisfy. The tree was at 26 errors and 7 warnings because CI only ran `next build` and never invoked the lint script that already existed. Add a Lint & Test job running eslint --max-warnings=0 and vitest, split from Build so failures report without waiting on the production bundle. --- .github/workflows/ci.yml | 29 +++++++ components/column/column-card.tsx | 78 +++++++++++++------ components/dialogs/version-history-dialog.tsx | 5 ++ eslint.config.mjs | 17 ++++ lib/columns/plugins/crates/client.tsx | 2 +- lib/columns/plugins/npm/client.tsx | 4 +- lib/columns/plugins/producthunt/client.tsx | 5 -- lib/columns/plugins/pypi/client.tsx | 2 +- 8 files changed, 108 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ce3150..8575483 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,35 @@ permissions: contents: read jobs: + # Split from Build so a lint or test failure reports in ~1min instead of + # waiting on the full Next production bundle. The npm cache is shared, so the + # duplicated install costs little. + check: + name: Lint & Test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-node@v7 + with: + # Matches the Dockerfile's node:22-alpine, so CI checks what ships. + node-version: 22 + cache: npm + + - name: Install dependencies + run: npm ci + + # `eslint` with no args lints the whole project under the flat config. + # `--max-warnings=0` keeps the tree at zero warnings now that it is + # clean — a new warning is a failure rather than scrollback noise. + - name: Lint + run: npm run lint -- --max-warnings=0 + + # Vitest needs no database: the suite runs against PGlite/in-memory + # fixtures, same as the build's fallback driver. + - name: Test + run: npm test + build: name: Build runs-on: ubuntu-latest diff --git a/components/column/column-card.tsx b/components/column/column-card.tsx index ba9ad53..a47f852 100644 --- a/components/column/column-card.tsx +++ b/components/column/column-card.tsx @@ -103,28 +103,6 @@ export function ColumnCard({ column }: { column: Column }) { const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ id: column.id }); - if (!type) { - return ( -
-

Unknown column type

-

- Type {column.typeId} is not registered. -

- -
- ); - } - - const Icon = type.icon; - const ItemRenderer = type.ItemRenderer; - const paginated = type?.capabilities?.paginated === true; const alertTerms = useMemo( @@ -184,6 +162,12 @@ export function ColumnCard({ column }: { column: Column }) { // visually but never clears the query, so re-opening shows what they last // typed (until they reload or manually clear). useEffect(() => { + // set-state-in-effect is suppressed, not fixed: the open state is sticky + // by design, so it can't be derived as `searchOpen || searchActive` — + // that would auto-close the row the moment the query is cleared, which is + // exactly the behaviour the note below rules out. The cascade is one + // bounded extra render on the mount-with-existing-query path. + // eslint-disable-next-line react-hooks/set-state-in-effect if (searchActive && !searchOpen) setSearchOpen(true); // intentionally only react to searchActive flipping true — don't auto- // close when the operator clears the query mid-type; let them keep the @@ -197,6 +181,10 @@ export function ColumnCard({ column }: { column: Column }) { // re-fires cleanly. useEffect(() => { if (pendingSearchOpen !== column.id) return; + // Syncing local state from an external store signal is effect-shaped by + // nature — the keypress happens outside this component and is routed here + // through the store, so there is no render-time value to derive from. + // eslint-disable-next-line react-hooks/set-state-in-effect setSearchOpen(true); requestAnimationFrame(() => searchInputRef.current?.focus()); clearPendingSearchOpen(column.id); @@ -206,8 +194,15 @@ export function ColumnCard({ column }: { column: Column }) { // the latest typeId/config without forcing a tear-down on every config edit. const typeIdRef = useRef(column.typeId); const configRef = useRef(column.config); - typeIdRef.current = column.typeId; - configRef.current = column.config; + // Synced in an effect rather than during render: mutating a ref mid-render + // is unsafe under concurrent rendering (a render that never commits would + // still have written). The useRef seeds hold the correct first-render values + // and this effect runs before the interval below can ever fire, so the + // closure still reads current values on every tick. + useEffect(() => { + typeIdRef.current = column.typeId; + configRef.current = column.config; + }); useEffect(() => { const intervalSeconds = column.refreshIntervalSeconds; @@ -297,7 +292,12 @@ export function ColumnCard({ column }: { column: Column }) { // on a fresh function reference and tear down its own subscription // mid-tick). const onRefreshRef = useRef(onRefresh); - onRefreshRef.current = onRefresh; + // Synced post-commit for the same reason as the config refs above; the + // refresh effect below only dereferences `.current` asynchronously, inside + // the pending-refresh branch, so it never reads a stale frame. + useEffect(() => { + onRefreshRef.current = onRefresh; + }); const refreshInFlightRef = useRef(false); useEffect(() => { if (!isPendingRefresh) return; @@ -310,6 +310,34 @@ export function ColumnCard({ column }: { column: Column }) { }); }, [isPendingRefresh, isFetching, column.id, clearPendingRefresh]); + // Unknown-type bail-out sits below every hook on purpose. A column's type + // resolves through the plugin registry, so `type` can flip from undefined to + // defined (or back) across renders as plugins register — returning early + // above the hooks would change hook order between renders and throw. The + // hooks above all tolerate a missing `type`: `paginated` optional-chains and + // the refresh effect guards with its own `if (!type) return`. + if (!type) { + return ( +
+

Unknown column type

+

+ Type {column.typeId} is not registered. +

+ +
+ ); + } + + const Icon = type.icon; + const ItemRenderer = type.ItemRenderer; + async function onLoadMore() { if (!paginated) return; setIsLoadingMore(true); diff --git a/components/dialogs/version-history-dialog.tsx b/components/dialogs/version-history-dialog.tsx index b6c56ea..dd0b647 100644 --- a/components/dialogs/version-history-dialog.tsx +++ b/components/dialogs/version-history-dialog.tsx @@ -35,6 +35,11 @@ export function VersionHistoryDialog({ deckId, open, onOpenChange }: Props) { useEffect(() => { if (!open || !deckId) return; let cancelled = false; + // Entering the loading state as the fetch is kicked off is the point of + // this effect — the data lives outside React and only the dialog opening + // can trigger the read. Removing the cascade would mean moving snapshot + // loading behind Suspense, which is out of scope here. + // eslint-disable-next-line react-hooks/set-state-in-effect setLoading(true); loadDeckSnapshots(deckId) .then((rows) => { diff --git a/eslint.config.mjs b/eslint.config.mjs index 05e726d..a3dfb7f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -13,6 +13,23 @@ const eslintConfig = defineConfig([ "build/**", "next-env.d.ts", ]), + { + rules: { + // Treat a leading underscore as "deliberately unused". Plugin ConfigForm + // components that take no configuration still declare their props + // parameter as `_` to document the contract they satisfy, and destructured + // rest-siblings are the idiomatic way to drop a key from an object. + "@typescript-eslint/no-unused-vars": [ + "warn", + { + argsIgnorePattern: "^_", + varsIgnorePattern: "^_", + caughtErrorsIgnorePattern: "^_", + ignoreRestSiblings: true, + }, + ], + }, + }, ]); export default eslintConfig; diff --git a/lib/columns/plugins/crates/client.tsx b/lib/columns/plugins/crates/client.tsx index e31f3ed..4dfa7a2 100644 --- a/lib/columns/plugins/crates/client.tsx +++ b/lib/columns/plugins/crates/client.tsx @@ -58,7 +58,7 @@ function ConfigForm({ value, onChange }: ConfigFormProps) {

Trending tracks the last 90 days of downloads — the - best signal for "what's hot right now."{" "} + best signal for “what’s hot right now.”{" "} Recently updated surfaces crates with active maintenance.

diff --git a/lib/columns/plugins/npm/client.tsx b/lib/columns/plugins/npm/client.tsx index 2112b98..05915c5 100644 --- a/lib/columns/plugins/npm/client.tsx +++ b/lib/columns/plugins/npm/client.tsx @@ -31,7 +31,7 @@ function ConfigForm({ value, onChange }: ConfigFormProps) { onChange={(e) => onChange({ ...value, query: e.target.value })} />

- npm's search endpoint requires a query; leave as javascript + npm’s search endpoint requires a query; leave as javascript {" "}for a broad popularity stream, or scope to a keyword (single word for substring match across name/description/keywords).

@@ -56,7 +56,7 @@ function ConfigForm({ value, onChange }: ConfigFormProps) {

Heavy-weights the chosen axis to 0.8; Combined{" "} - mirrors npm's default ranking blend. + mirrors npm’s default ranking blend.

diff --git a/lib/columns/plugins/producthunt/client.tsx b/lib/columns/plugins/producthunt/client.tsx index 0a069e1..8db8d70 100644 --- a/lib/columns/plugins/producthunt/client.tsx +++ b/lib/columns/plugins/producthunt/client.tsx @@ -22,11 +22,6 @@ import { type ProductHuntMeta, } from "./plugin"; -const MODE_LABELS: Record = { - today: "Today's launches", - topic: "Filter by topic", -}; - function ConfigForm({ value, onChange }: ConfigFormProps) { return (
diff --git a/lib/columns/plugins/pypi/client.tsx b/lib/columns/plugins/pypi/client.tsx index af8527d..b3b1255 100644 --- a/lib/columns/plugins/pypi/client.tsx +++ b/lib/columns/plugins/pypi/client.tsx @@ -40,7 +40,7 @@ function ConfigForm({ value, onChange }: ConfigFormProps) {

- Updates and New packages read PyPI's + Updates and New packages read PyPI’s public RSS feeds (last ~40 entries, time-ordered).{" "} Top · 30d ranks the top 8000 packages by 30-day downloads via the community-maintained mirror — no API key required.