Repository navigation
chore: adopt skeleton's lint, typecheck, test, token and CI setup - #570
Merged
Merged
Conversation
saveExact stops pnpm add from writing ranges, and country-flag-icons loses the one caret. trustPolicy refuses a version published with weaker provenance than an earlier one. allowBuilds lets only esbuild and msw run install scripts: core-js is no longer installed and @tailwindcss/oxide has no install script. pnpm settings move to pnpm-workspace.yaml, the only place pnpm 11 and later read them, so the package.json pnpm field and .npmrc go. pnpm writes pnpm-lock.yaml back in its own format, undoing the Prettier pass it got in #558; no resolved version changes. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
pnpm update and pnpm dedupe move every transitive dependency to the newest version its parents allow, skipping releases under a day old. Direct dependencies are exact, so none of them move. pnpm audit drops from 126 advisories to 20. Each of the 20 is fixed only by bumping a direct dependency: echarts, zod and @hey-api/openapi-ts themselves, or the vitest, storybook, typescript-eslint and @tanstack/router-plugin releases that pull in the vulnerable package. semver 6.3.1 goes too. It was published without provenance after earlier releases carried it, so the no-downgrade trust policy rejects it; 6.3.2 has a trusted publisher. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
Node 26.13.0 is the active LTS and the version skeleton pins; 24.13.0 predates four security releases on the 24 line. pnpm 12.18.0 matches skeleton too, and adds what pnpm 10 lacks: it checks the whole lockfile against the trust policy on every install, fails when a dependency has an install script that allowBuilds does not list, skips releases under a day old, and rejects settings it does not recognise. Playwright moves to skeleton's 1.65.0 with it: 1.58.2 hangs unzipping its browser on Node 26, and 1.65.0 also has builds for Ubuntu 26.04. package.json is the one place the Node and pnpm versions live: packageManager for pnpm and Corepack, and volta for Volta and for setup-node's node-version-file. .nvmrc goes. pnpm 12 records its own integrity in a second document at the top of pnpm-lock.yaml.
knip 6.44.2, skeleton's version, runs as pnpm knip. Routes are its entries; its Vite, Vitest and Storybook plugins add main.tsx, the test setups and the stories. Its Tailwind and Storybook plugins read imports out of CSS (@import and @plugin) and MDX, so knip.ts does not override them with skeleton's narrower regexes, which miss @plugin. The generated API client is ignored. The script sets KNIP_DISABLE_RAW_TRANSFER: otherwise knip's parser reserves 6 GiB of address space, which Linux refuses on a 4 GB machine. @eslint/css, cron-parser and eslint-plugin-n are imported nowhere and go. country-flag-icons, read through import.meta.glob, and @commitlint/cli, run by hand, stay listed in ignoreDependencies. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
knip --fix removed what nothing imports: 36 files, 16 of them component barrels whose components only stories and design-sync use, plus the live page's dead BlockDetailsCard, DataAvailability and slot-progress hook and utils. Barrels now re-export only what other modules import, as skeleton's do. Declarations left unused once their export went were deleted. Two exceptions are tagged @public: the useSlotPlayer barrel keeps the hooks its JSDoc guides import, and Input keeps exporting its parts so the module still exports components for Fast Refresh. ReleaseStatus now types the status sets instead of being dropped. design-sync's entry loses the imports of pages that no longer exist, and its tsconfig.paths.json the 104 aliases whose index files are gone. The create-new-component skill's barrel step exports only the component and runs pnpm knip. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
test:coverage ran Vitest's default config, the Storybook browser one,
in watch mode, without a coverage provider installed. It now runs the
unit config once with @vitest/coverage-v8 4.0.18, which must match
vitest's version. The report covers src/**/*.{ts,tsx} minus generated
code, tests, stories and type declarations, the include and exclude
list skeleton uses, including its test-utils, design-docs and styles
paths. coverage/ is gitignored.
Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
format and format:check now cover the whole repo, as skeleton's do,
instead of src/**/*.{ts,tsx}, which ESLint already formats. The other
files are formatted here: Markdown, JSON, YAML and index.html.
.prettierignore skips what tools write: pnpm-lock.yaml, the generated
API client and route tree, msw's worker, the map data and design-sync's
files. Markdown code blocks are left as written, since docs quote
fragments such as {formatSlot(slot)} that Prettier would rewrite as
statements. test-results/.last-run.json, a Playwright artifact committed
by accident, is deleted and test-results/ ignored.
Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
Every action is pinned to a commit SHA with its version beside it, at skeleton's versions where skeleton uses the action: checkout v7.0.1, pnpm/action-setup v6.1.1, setup-node v7.1.0, actions/cache v6.1.0. Node comes from package.json through node-version-file. Each job has a timeout and checks out without persisting the token, except the alpha release, which pushes a tag. The release workflows install without a dependency cache, so their archives cannot pick up a poisoned one, and release.yaml reads the release tag from env, not a template. Vet cancels a superseded run of the same PR or branch and runs on master too, so pull requests can restore the caches master saves. Lint also runs format:check and knip; Build Storybook is a new job; story tests cache Playwright's browser under its version and run on ubuntu-latest again, which Playwright 1.65 has builds for. The invalid branches filter on workflow_dispatch goes. deploy-storybook replaces the Bitovi action, which checked out again and called unpinned actions inside, with a frozen install, the build and the pinned Pages upload and deploy actions. A newer push queues behind a running deploy instead of cancelling it.
The commands section and the pre-commit checklist name the new format:check, knip and test:coverage scripts, which CI now runs except for coverage. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-90a0-703b-95c6-9ded2f98b137
Config, bounds, the archive downloads and the gas profiler are served by lab-backend itself, not the CBT API, so src/api has no options for them. openapi/lab-backend.yaml describes those routes as the backend serves them today (checked against production responses and the injected page), and pnpm generate:api now also writes src/lab-api from it. The lab-backend client is separate from the CBT one, so NetworkProvider's network rewrite never touches paths that already name their network. Its schemas use classic zod: zod/mini in the entry chunk would stop the minifier dropping unused CBT schemas and cost that chunk 25 kB gzipped. Both jobs use postProcess, which replaces the deprecated format and lint options with identical output. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-a7a0-71f2-ac45-b57abc76f51a
useConfig and useBounds spread getConfigOptions and getBoundsOptions instead of hand-rolled fetches, so their responses are validated against the spec. One createQueryClient replaces the client built inside __root.tsx and its three providers: main.tsx provides it once above the router, and Storybook and the tests use it with their own defaults. Its staleTime is 30 s, so a remount or a window focus within that reads the cache; pages that follow the chain head already set their own. It keeps the infinite retry with backoff, and seeds config and bounds from the page lab-backend injects, now validated and under the generated keys. The generated client throws the response body, so ConfigGate shows lab-backend's message through apiErrorMessage. A network configured outside cartographoor has null consensus forks, which the spec now says, so the fork helpers guard for it. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-a7a0-71f2-ac45-b57abc76f51a
…ient useArchiveDownload runs the generated downloadBlock and downloadBlob in a useMutation instead of fetching hand-built URLs, so the URL builders in slot-download.ts go. It asks for a blob, so a JSON block is saved as a file rather than parsed, and shows lab-backend's error message as before. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-a7a0-71f2-ac45-b57abc76f51a
…h the lab-backend client useGasSchedule spreads getGasScheduleOptions and SimulatePage runs simulateBlockMutation, replacing two hand-rolled fetches. The response type comes from the spec, which drops baseFork and customSchedule: the backend never sends them and nothing read them. Errors and the syncing notice read lab-backend's message through apiErrorMessage. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-a7a0-71f2-ac45-b57abc76f51a
Copying a card fetched its own PNG data URL to turn it into a Blob. dataUrlToBlob decodes the data URL in place instead. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-a7a0-71f2-ac45-b57abc76f51a
Adds skeleton's data rules, adapted to Lab's two generated clients: query options for reads, mutations for actions, never fetch; new lab-backend routes go into openapi/lab-backend.yaml; createQueryClient for the app, stories and tests; config and bounds through their hooks, never the window globals. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-a7a0-71f2-ac45-b57abc76f51a
… them Every design token now lives in src/styles/tokens.css, following savid/skeleton. Tailwind's defaults are reset (--*: initial) and the scales Lab uses are declared explicitly, so only tokens compile. Each base colour is a light-dark(light, dark) pair with its own star value in html.star; tints, washes and hovers derive from the base colours, so the dark: and star: variants are gone. Category colours keep Tailwind's oklch values, and all three themes render as before. eslint-plugin-better-tailwindcss 4.11.0 enforces the tokens: correctness-error with entryPoint, canonical classes, and no-restricted-classes for arbitrary values, dark: variants, leading-* and opacity modifiers. It also reads Lab's class constants, ...ClassName props, get...Class helpers and the router's activeProps. Colour literals in style and SVG colour attributes are rejected, which retires the no-hardcoded-colors and no-primitive-color-scales rules. tokens.test.ts checks the file's shape, every colour's group and tints, star's palette, contrast in all three themes, and the browser theme colours, now the background of each theme (dark kept a stale grey). contrast.ts and its test are skeleton's. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7d-2d84-729c-a47a-2756cea5d2ae
src/design-docs adds skeleton's Foundations pages: Introduction, Colours, Typography and Layout. Every value on them is read from tokens.css through src/styles/tokenSource.ts: each base colour in light, dark and star, every tint, derived, category and data colour, the contrast rules with their ratios, the type scale, weights, tracking and line heights, and spacing, widths, radii, breakpoints, shadows, easing and animations. Storybook sorts the pages first. docs.test.ts checks that each page shows only stories its stories file exports, and that every token namespace in tokens.css appears on a page.
axe, run on every story, flags these outside colour contrast: - Dropdown: an element trigger is the menu button itself, so its ARIA attributes no longer sit on a wrapper div (aria-allowed-attr) - CheckboxGroup: label and describe each checkbox through Headless UI's Field, Label and Description; a native label does not name its span - RadioGroup and ListContainer: radio options sit in a div, not a list; ListSection renders a list item holding a nested list - Stats and MiniStat: dt and dd only inside a dl, with nothing else between them - Icon-only buttons get names: table pagination, the probe details button, the state-expiry info popover - Timestamp no longer sets "[object Object]" as the name of custom content; ScrollAnchor's name is its content, which it shows - ScrollArea's viewport takes focus so keyboard users can scroll it - Alert titles are h2 and SlotTimeline drops its hidden h4, so heading levels do not skip; the mobile sidebar dialog is named - Client logos beside a client's name are decorative, and a logo inside a filter button is not a link; the probes view's inline link is underlined Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-9df8-72f3-bbbc-2c7359b5fbec
zrender paints a large point set over several animation frames and keeps going after ReactECharts disposes the chart, so the next frame throws "Cannot read properties of null (reading '0')". Leaving a page while the map was still drawing thousands of points logged that error, and it made Map2D's story tests flaky. Map2D now clears the chart in a layout effect cleanup, which runs before ReactECharts unmounts. Clearing repaints at once and ends the pending paint. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-9df8-72f3-bbbc-2c7359b5fbec
Brings over savid/skeleton's web test setup: - One vitest.config.ts with a unit project (jsdom) and a storybook project that runs every story in Chromium once per theme (light, dark, star), in UTC, en-US and reduced motion. restoreMocks and unstubGlobals are on. - @storybook/addon-a11y fails a story on any violation. Colour contrast is off until the theme tokens meet it. A console error or an API request no story mocked also fails it. - MSW 3 serves its worker through the Vite plugin, so public/mockServiceWorker.js is gone from the app build. Storybook builds from .storybook/vite.config.ts instead of the app's Vite config. - The storybook project serves no index.html: a story that asks for a missing file gets a 404 instead of the app, whose entry pulled every route into the run and reloaded the page mid-test. RadioGroup and Card stories asked for four images that do not exist; they use ones that do. - @/test-utils: render and renderHook with a fresh query client and a router, the MSW server (an unmocked request fails the test), shared handlers per API domain with success, failure and pending states that replace .storybook/mocks.ts, and NOW_MS for the current time. - Unit tests move onto it; the hooks that mocked fetch now go through MSW. - Stories that never rendered now do: Breadcrumb builds a real router, the preview adds NotificationProvider as the app root does (for CopyToClipboard and every PopoutCard menu), and PolicySelector and the probes view get the props they require. FatalError and ConfigGate stop logging the error they already show. Bumps storybook to 10.6.6, vitest to 5.1.0, msw to 3.0.4, msw-storybook-addon to 3.0.3, playwright to 1.65.0, @testing-library/react to 16.3.3 and @testing-library/jest-dom to 7.0.1, matching skeleton. Amp-Thread-ID: https://ampcode.com/threads/T-01a1cab4-9df8-72f3-bbbc-2c7359b5fbec
Rewrite the parent-relative imports in src and .storybook to the @/ alias and make no-restricted-imports reject new ones. The design-sync bundle keeps its own path map, so .design-sync is exempt. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
typecheck now runs tsc -b over three projects, as skeleton does: the app (tsconfig.app.json, now with noUncheckedIndexedAccess and the test-only paths excluded), a new tsconfig.test.json for tests, stories, test-utils, design docs, token tooling and the Storybook preview, and tsconfig.node.json for the Vite, Vitest, knip and Storybook configs. Before, typecheck ran tsc --noEmit on a solution file with no sources, so it checked nothing, and tests and stories were never type-checked at all. baseUrl is gone: with it, `import 'knip'` in knip.ts resolved to knip.ts itself. The ~750 errors this surfaced are fixed in the code, without non-null assertions, casts or suppressions. Missing entries get the value the code means (0, '', no row) or an early return; tables built with createColumnHelper become plain DataTableColumn objects, as other tables already were; stories pass the props their components require. Fixing the types also fixed a few real defects: one-value box plots no longer get NaN quartiles, hasUseEffectEvent stops calling require() in ESM, the BlobDataAvailability stories cut their data at the time they are named after, and the Sidebar story's collapse button works. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Take savid/skeleton's ESLint setup and package versions: eslint 10.14.0, typescript-eslint 8.72.2, @eslint-react/eslint-plugin 5.28.8 (its recommended-typescript config replaces eslint-plugin-react), eslint-plugin-react-hooks 7.1.1, @vitest/eslint-plugin 1.6.28 and the matching prettier, react-refresh and globals releases. The config moves to defineConfig with skeleton's layout. The TSAnyKeyword selector duplicated @typescript-eslint/no-explicit-any and is gone; @eslint/compat had no users left. The rules this turns up violations of stay off here and come on, with their fixes, in the commits that follow. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Render contexts as providers directly, read them with use(), and take ref as a prop instead of forwardRef. @eslint-react flags each old form. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Lint now runs with --max-warnings 0 and reportUnusedDisableDirectives, and exhaustive-deps is an error, as in skeleton. The warnings it turned up are fixed: useTraceSpans recomputes when payload or PTC data change, and GasHistogram's bucket presets and types move out of the component file so it stays fast-refreshable. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Turn on eslint-plugin-react-hooks' React Compiler rules at error, as skeleton does, with @eslint-react's purity, set-state-in-effect, static-components and use-memo rules, and fix the 179 violations they found in 53 files. The compiler is not enabled in the build; the rules are a lint-time guard. State that copied props or other state in an effect is now derived during render, or reset by comparing with the previous value; components defined inside components moved to module level; Date.now() during render became a ticking clock (xatu nodes' useNow, ForkCountdown) or a captured value; ref reads during render moved to state or effects; external stores use useSyncExternalStore. Behaviour that changes with it: - Checkbox ids are stable (useId) instead of new on every render. - The live slot view derives what it shows from the playhead, so it no longer lags a frame, updates while paused and drops columns on a seek back. - Xatu nodes pages share one clock that ticks every minute, so offline status and bucket ranges move on time. - SlotPlayerProvider resumes at the live edge from its animation loop rather than an effect; new tests cover pause, resume and clamping. - useTabState only follows the URL when the URL's tab changes, so a click no longer snaps back while the router catches up. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Turn on the rest of @eslint-react's recommended rules and fix what they found: 84 array-index keys, 15 refs not named …Ref, 16 useState calls with an eager initializer or an unmatched setter name, and the cloneElement and Children.toArray uses. Lists are keyed by what identifies an item in the data (block number, slot, address, probe sort key, client and version); identical skeleton placeholders are written out instead of mapped over an index. Button sizes its icons from a box-less wrapper and ButtonGroup shapes its children with *:first/*:last variants instead of rewriting their props. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Every fixed value in an inline style becomes a Tailwind class, so a style prop only carries values computed at runtime (heights passed as props, positions, data colours). 108 values in 43 files; the CSS they produce is unchanged, checked element by element in Chromium. Values no token covered become named utilities in utilities.css (vertical writing mode, viewport-sized type, the swim-lane label size, a 1px radius, animation durations and the two performance legend gradients). echarts-for-react writes a default inline height, so its fixed heights use the important modifier (h-70!). Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Turn on skeleton's test-file rules: a focused or skipped test fails lint, and tests import Testing Library through @/test-utils, never @testing-library/react directly. The eight skipped tests run again. Seven CopyToClipboard tests mocked a navigator.clipboard that userEvent.setup() then replaced with its own, so they now spy on the clipboard userEvent installs. The zero-delay useInterval test runs on real timers, because fake timers fire a 0 ms interval forever. useArchiveDownload's test goes through MSW instead of stubbing fetch. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Twenty story files drew their data from Math.random(), so a story showed different data on every run, under every theme and in every screenshot. They now use seededRandom from @/test-utils/random with a fixed seed per file, or per render where the data is built while rendering. Lint rejects Math.random and Date.now in tests and stories, which take random data and the time from @/test-utils. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
index.html's critical CSS reset margin, padding, font and overflow on everything in the page until ConfigGate adds react-loaded. With the configuration lab-backend injects, NetworkProvider now picks the network during render, so pages mount in that first commit and lay out under the reset: on /ethereum/live at 1440x900 the map canvas started 1079px wide in its 911px column and stayed that way. The loading screen is an empty div, so only the overlay keeps its styles. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
The 'prettier' post-process preset reads .prettierignore, which lists src/api and src/lab-api to keep them out of format:check, so pnpm generate:api wrote them unformatted: a diff of most of each file. Run Prettier with only .gitignore instead; src/lab-api now regenerates byte for byte. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
An injected configuration or bounds that failed zConfig/zBounds was dropped, and the fetched copy then failed the client's response validation, so retry: Infinity kept ConfigGate on "Loading Lab..." for good with nothing logged. One network without genesis_delay, or disabled_networks: null (a Go nil slice), was enough. Master renders both. The lab-backend spec is written by hand, so its client no longer validates requests or responses, and the injected configuration and bounds seed the cache as they are. ConfigGate drops its error line: it never rendered, since retries keep error null and the config route answers errors in plain text. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
create-new-component still taught forwardRef, cloneElement for icons, text-error (the token is danger), an arbitrary min-w-[600px], an index key and exported types nothing imports, so a component built from it failed lint and knip. It now passes ref as a prop, sizes icons from a wrapper as Button does, and uses token classes only. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Most of these re-tested TanStack Query: loading flags, caching, selectors, refetch. All 20 useConfig tests passed with staleTime and gcTime set to 0. Kept: the config request and its result, a failed refetch keeping the cached config, no bounds request without a network, a refetch on a network switch, useTablesBounds' tables and aggregate with a missing table and with none, and useNetworksBounds sharing the cache with useBounds. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Lab already depends on culori, whose wcagContrast reads hex and oklch, so contrast.ts, its tests and the srgbValue conversion go. The theme colour check compares formatHex8 values instead of sameColor. The oklch tag colours now measure before clamping to sRGB, which moves their ratios by up to 0.2; none crosses its minimum. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
The comment said the minimums hold today's values and can only rise. They sit just under the light theme's weakest pair; dark and star clear them by far, so they don't hold those themes in place. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Skeleton's useNow, which ticks every intervalMs, replaces the home page's local clock, ForkCountdown's interval and the xatu nodes pages' own hook and its test. The nodes pages tick every minute, the others every second, as before. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
no-array-index-key had 15 *Skeleton.tsx files write identical placeholders out by hand, such as StateExpirySkeleton's 10 rows and DataAvailabilitySkeleton's 12 heatmap rows, and CardChain dress its index up as a key. Placeholders never reorder, so the rule is off for *Skeleton.tsx, those files map over Array.from again, and CardChain keys its placeholders by index with a reasoned disable. Lists of data keep their identity keys. Three class pairs in the touched files become their canonical forms (inset-x-1, size-full, size-4). Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
setup.ts set the clients' base URL in beforeAll, after test modules had loaded, so a query key built at module level lacked it and missed the cache. Setting it at the top of the file lets keys be built anywhere, and the rule telling authors to build them inside tests goes. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
boundsHandlers.failure and .pending had no test or story. The rule now asks for a handler for each state a test or story uses, not one per state. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
…gain No behaviour change called for dataUrlToBlob, so PopoutCard fetches its data URL as on master, and the helper and its test go. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
useArchiveDownload's test already covers apiErrorMessage's two outcomes, and the Breadcrumb test mocked the router to recheck what the Breadcrumb stories check with a real one. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
SlotPlayerProvider logged its maxSlot inputs to the console on every bounds change, in production too; the test spied on console.log only to silence it. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
It failed CI whenever a namespace lacked a Foundations story, which is a docs gap, not a defect. The check that each page shows only stories its stories file exports stays. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
The message claimed opacity modifiers skip the contrast checks, but the tint tokens that replace them are not contrast-checked either. Like an arbitrary value, a modifier bypasses the tokens: every tint is a named token in tokens.css. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
![redpandabot[bot]](https://avatars.githubusercontent.com/in/3749543?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR adopts skeleton's toolchain end to end: stricter ESLint (React Compiler + exhaustive-deps at error, @/ imports), a single tokens.css with lint-enforced token classes, knip, pinned/hardened CI, a Storybook a11y and light/dark/star matrix, and a large pass of React 19 API migrations; after the last review, 16 commits revert several over-refactors to master's approach and tighten tests and docs. The mechanical changes I re-checked (skeleton re-mapping, class-canonicalisation, useNow, PopoutCard, query-client seeding) hold up, but the SlotPlayerProvider rewrite trades a cancelled rAF for a continuously spinning one while paused at the live edge.
Issues
- 🟡
src/providers/SlotPlayerProvider/SlotPlayerProvider.tsx:545— playback loop spins a no-op rAF the whole time it waits at the live edge — see the thread on that line
Reviewed @ fb279ef2
"Real artists ship." — Steve Jobs
| // Auto-resume when newer slots arrive past the live edge boundary in continuous mode | ||
| if (currentSlotRef.current >= maxSlotRef.current) { | ||
| lastProcessedTime = currentTime; | ||
| animationFrameRef.current = requestAnimationFrame(animate); |
There was a problem hiding this comment.
🟡 playback loop spins a no-op rAF the whole time it waits at the live edge
The boundary wait used to cancel the animation frame (the old effect cancelled whenever !isPlaying) and a small effect resumed it when maxSlot advanced; now the loop re-registers requestAnimationFrame(animate) every frame while currentSlotRef >= maxSlotRef, doing nothing but reading two refs. The live route mounts with initialPlaying={true} in continuous mode (src/routes/ethereum/live.tsx:20), so after the player catches up it sits at this boundary most of the time, keeping a 60 fps callback (and the tab out of idle) for as long as the page is open — a battery/CPU cost the state-driven resume did not have.
Member
Author
There was a problem hiding this comment.
Fixed in 2a1f5ae. The loop now stops while it waits at the live edge and restarts on the render that brings newer slots, so no frame runs during the wait. The live-edge test now checks that no frame is pending while it waits; that check fails on the previous code.
There was a problem hiding this comment.
This was my finding, and 2a1f5ae does address it. canResumeAtBoundary now also requires currentSlot < maxSlot (src/providers/SlotPlayerProvider/SlotPlayerProvider.tsx:508-509), so once the boundary pause is set the layout effect takes the cancel-and-return path (src/providers/SlotPlayerProvider/SlotPlayerProvider.tsx:526-532) rather than scheduling a frame. The loop only restarts on the render that raises maxSlot past currentSlot, through the effect's canResumeAtBoundary dependency (src/providers/SlotPlayerProvider/SlotPlayerProvider.tsx:609).
The remaining no-op branch at src/providers/SlotPlayerProvider/SlotPlayerProvider.tsx:544-547 is now only reachable for the first frame after that restart, before maxSlotRef catches up in its passive effect (src/providers/SlotPlayerProvider/SlotPlayerProvider.tsx:435-437) — not for the entire wait at the live edge. The new assertion in src/providers/SlotPlayerProvider/SlotPlayerProvider.test.tsx:110 covers the waiting state.
@ fad6f5b3
The playback loop kept requesting a frame every 16 ms while paused at the live edge in continuous mode, only to check whether newer slots had arrived. It now stops there and restarts on the render that brings newer slots, as master's effect-driven resume did, so a page waiting on stalled data no longer runs a no-op loop. The live-edge test checks that no frame is pending while it waits. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
The slot timeline set its row and bar heights from ROW_HEIGHT in style, fixed values the inline-style lint can't see behind a constant. They are h-7 and h-5 now, the same 28px and 20px. LABEL_WIDTH stays a constant: it aligns the header, the grid and the label column. styling.md said lint rejects fixed values in style; it rejects literals, so it now says a constant is the author's to turn into a class. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Its width was `${(4 / 12) * 100}%` in style: a fixed value worked out
from literals, which the inline-style lint can't see. w-1/3 is the same
width.
Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Member
Author
Partly fixed:
Left as they are:
|
Member
Author
Not changing this. The hook is now the shared |
There was a problem hiding this comment.
Adopts skeleton's web toolchain: stricter ESLint (10.14, @eslint-react, max-warnings 0), token-only styling with a rebuilt tokens.css, CI hardening with SHA-pinned actions, knip, Storybook a11y/theme matrix, plus the mechanical @/ import rewrite and React Compiler fixes. The three commits since the last review fix the open finding — the playback loop now stops at the live edge and restarts on the next render where bounds grow, with no livelock path since the loop's ref guard only waits for the passive ref sync — and replace fixed inline sizes with equivalent token classes (28px → h-7, 20px → h-5, 33.3% → w-1/3). No new problems found; bumped dependency versions query clean on OSV.
Reviewed 1028 changed file(s) @ fad6f5b3 — no blocking issues found.
"Real artists ship." — Steve Jobs
Browsers throttle timers in hidden tabs, so useNow could hold a value a minute or more old when a tab came back, until its next tick. It now also reads the clock on visibilitychange, so the xatu nodes pages' offline status and ranges catch up as soon as the tab is shown. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
Member
Author
Now done in e5fab34, replacing my earlier reply: the shared |
enforce-canonical-classes went quiet partway through a full lint run (see the next commit), so pairs such as h-5 w-5, px-2 py-2, h-full w-full and right-15 left-15 got through in 34 files. eslint --fix gave them their canonical forms: size-5, p-2, size-full, inset-x-15. Each pair and its replacement set the same properties to the same values. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
… classes
On Tailwind 4.1.18, canonicalizeCandidates gave wrong answers once it
had run many times on one design system: after a few hundred class
lists, h-full w-full no longer collapsed to size-full. ESLint's
better-tailwindcss plugin keeps one design system for the whole run, so
enforce-canonical-classes stopped reporting partway through pnpm lint
and CI, while linting a single file still caught everything. Tailwind
fixed this in 4.2.0 ("Fix incorrect canonicalization results when
canonicalizing multiple times", tailwindlabs/tailwindcss#19675).
4.3.5 is skeleton's version. The built CSS is equivalent: spacing
values are written 0 and var(--spacing) instead of calc(), group
variants use another selector form with the same specificity, and the
preflight changes are ones Lab's font tokens override.
Amp-Thread-ID: https://ampcode.com/threads/T-01a1ca7c-dd8d-7478-afd0-5a9660d5f2b3
|
The three commits since the last review are safe to take: the canonical Tailwind rewrite ( Partial review: 1028 of 1029 changed files included. Skipped 1 file(s) whose individual diff exceeds 500000 bytes:
No findings in the reviewed scope; no whole-PR approval issued. |
samcm
approved these changes
Nov 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings every item from the review of savid/skeleton's web setup to Lab, reusing skeleton's configs, rules, code and package versions where they fit. Much of the diff is mechanical (the
@/import rewrite, repo-wide formatting, the token class migration), so it reads more easily commit by commit.What changed
1. Stricter ESLint
@eslint-reactrecommended-typescript replaceseslint-plugin-react;eslint-plugin-react-hooks7.1.1,@vitest/eslint-plugin1.6.28, typescript-eslint 8.72.2.--max-warnings 0andreportUnusedDisableDirectives: 'error'.no-restricted-importsbans../*; about 500 parent-relative imports in 238 files now go through@/.exhaustive-depsat error, with the rest of@eslint-react's recommended set: about 360 violations, fixed in the code, including React 19's context and ref APIs (<Context>,use(),refas a prop). The exception is loading placeholders, which never reorder:no-array-index-keyis off for*Skeleton.tsx, and CardChain keys its placeholders by index with a reasoned disable.@/test-utils, andDate.now/Math.randomare rejected in tests and stories.styleobject. Lint can't see a fixed value behind a constant or worked out from literals: the slot timeline's were made classes, and itsLABEL_WIDTHstays a constant because it aligns three components. SVG colour attributes on elements take onlycurrentColor,noneor aurl()paint, as in skeleton; on components, which use colour props as variant names (<Badge color="green">), only colour literals are rejected.2. CI hardening: actions pinned by SHA (skeleton's SHAs where it uses the action), superseded runs cancelled, timeouts and
persist-credentials: falseon every job,node-version-file: package.json, a Playwright browser cache, format, knip and Storybook-build checks. deploy-storybook builds with a frozen install and pinned Pages actions instead of the Bitovi action.3. knip: knip 6.44.2 in CI. It removed 36 unused files, every unused export and the unused
@eslint/css,cron-parserandeslint-plugin-n.4. Token enforcement:
src/styles/tokens.cssholds every token, colours aslight-dark()pairs with star as anhtml.staroverride. Tailwind's defaults are reset, so an unknown class fails lint, as dodark:variants (166 removed), arbitrary values (about 250 replaced),leading-*and opacity modifiers. Opacity modifiers give way to about 160 tint tokens (bg-primary-10). Tailwind is 4.3.5, skeleton's version: on 4.1.18, canonicalization went wrong after a few hundred calls (fixed upstream in 4.2.0), soenforce-canonical-classeswent quiet partway through every full lint. The 88 pairs it had missed in 34 files (h-5 w-5,px-2 py-2) are canonical now. Contrast (culori'swcagContrast) and token tests guard the file;.claude/rules/styling.mdreplacestheming.md.5. Storybook a11y and theme matrix:
@storybook/addon-a11yfails a story on any violation, every story runs in Chromium under light, dark and star (UTC, en-US, reduced motion), and a console error or an unmocked API request fails it. The violations it found are fixed in the shared components.6. Shared test-utils:
@/test-utils(render,renderHook, an MSW server that fails unmocked requests), typed fixtures with a handler for each state a test or story uses, shared by tests and stories,NOW_MS/freezeNow,seededRandom, and onevitest.config.tswithunitandstorybookprojects. MSW serves its worker through its Vite plugin, sopublic/mockServiceWorker.jsis gone. Items 5 and 6 bring the test stack's major bumps: vitest 4→5, msw 2→3, msw-storybook-addon 2→3, jsdom 28→31,@testing-library/jest-dom6→7 and Storybook 10.2→10.6.7. Data fetching on the generated client: lab-backend's own routes get a spec (
openapi/lab-backend.yaml) and a generated client insrc/lab-api; config, bounds, block and blob downloads, gas schedules and block simulation read through it. OnecreateQueryClientserves the app, Storybook and tests, seeded from the page's injected config and bounds. The lab-backend client validates neither requests nor responses: its spec is written by hand, and a field it gets wrong must not keep the app from loading. The CBT client validates both, as on master.8. Dependencies and supply chain: exact versions,
trustPolicy: no-downgrade, install scripts limited to esbuild, Node 26.13 and pnpm 12.18 withpackage.jsonas the single version source, and transitive dependencies refreshed.pnpm auditgoes from 126 advisories to 5.9. Typecheck:
typecheckrunstsc -bover the app, a newtsconfig.test.json(tests, stories, test-utils, design docs, the Storybook preview) and the config files, withnoUncheckedIndexedAccess. About 830 errors fixed without non-null assertions, casts or suppressions. Before,typecheckchecked nothing (a solution file with no sources) and tests and stories were never type-checked.10. Rules and agent guidance:
.claude/rulesgains loading-states, React and tests rules (skeleton's, adapted), with path front matter; CLAUDE.md lists the new commands and checks, and the create-new-component skill builds to the new rules.11. Design docs: Foundations pages (Introduction, Colours, Typography, Layout) generated from
tokens.css, first in Storybook's sidebar.12. Coverage:
@vitest/coverage-v85.1.0 with skeleton's include/exclude list;pnpm test:coverageworks.Bugs found and fixed on the way
useTraceSpansignored payload and PTC data in its memo, so the slot trace went stale (exhaustive-deps).useTabStatecould snap back to the previous tab while the router caught up with a click.NaNquartiles;hasUseEffectEventalways returnedfalse(requirein ESM).grid-cols-${n}class never compiled, so its labels stacked; the light RadioGroup stacked card showed white text on the card surface.userEvent.setup()replaced. They run again.min_client_versions,nullconsensus/service_urls).Behaviour changes worth a look
light-dark()needs them (Firefox stays at Tailwind's 128). Older browsers, iOS 16 Safari included, lose the theme colours; there is no@supportsfallback, by choice.smand up; on master their ownsm:padding (24px) won over the card rule.color-schemefollows the theme, so native controls are dark on dark and star; the dark browser theme colour is#0f0f0e.h2, the ScrollArea viewport takes focus, the probes view's inline link is underlined, and FatalError/ConfigGate no longerconsole.errorwhat they already show.useNowclock (skeleton's, plus a re-read when the tab is shown) drives the home page, ForkCountdown and the xatu nodes pages. The nodes pages tick each minute, so offline status and bucket ranges move on time.Left out, and why
@vitest/eslint-plugindeclares an optional TS ≥ 6 peer, so pnpm prints a peer warning.staleTime: Lab keeps master's default of 0, so a query refetches on remount and window focus unless it sets its own.FakeEventSource: Lab has no event stream..design-sync/ds-tailwind.cssis a generated snapshot; regenerate it per.design-sync/NOTES.md.Verification
Run on this branch in an orb with Node 26.13 and pnpm 12.18:
pnpm lint: 0 problems.pnpm typecheck: 0 errors.pnpm format:checkandpnpm knip: pass.A full
eslint .catches a non-canonical pair wherever it is: one planted in the 1,198th of 1,434 files is reported.Tailwind 4.1.18 → 4.3.5 with the canonical classes: computed styles match element for element on the home, forks and slot pages of a production build (82, 189 and 472 elements), and the built CSS differs only in equivalent forms.
pnpm test:unit: 74 files, 922 tests passed, none skipped (master skipped 8).pnpm buildandpnpm build-storybook: pass.Story tests in the light theme: 107 files and 978 tests passed; CI runs all three themes. Globe and Map stay tagged
test-exclude(WebGL).pnpm install --frozen-lockfilepasses pnpm 12's supply-chain check; commitlint passes on every commit.The app against production (
BACKEND=production pnpm dev): home, the live slot view, slots, xatu nodes, forks and the gas simulator render with live data and no console errors.A production build serving production's injected
__CONFIG__/__BOUNDS__, at 1440×900: the live map's canvas is created 911px wide in its 911px column (1079px before the index.html fix).The slot timeline (
/ethereum/slots/15635500) matches production: 33 rows of 28px, 20px bars, a 280px label column. The live view plays on, one slot every 12 s, with no console errors.A configuration that misses the spec (a network without
genesis_delay,disabled_networks: null) renders the app, whether injected or fetched; before, ConfigGate stayed on "Loading Lab..." in both cases.CI on this PR: Lint (with format and knip), Build, Build Storybook, Unit Test and both Storybook Test shards pass; the shards, which now run every story under three themes, take about 4 minutes each.
Not verified: the network page, which crashes headless Chromium in the orb on production too, and the release and Pages deploy workflows, which only run on a push to master.