From 3e37dcfb45fac9180420f354097bb2092adb3958 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 12:50:33 +0200 Subject: [PATCH] fix(client): lint:layers holds src/types.ts to the rules of types A file's layer was the first path segment under src/, so src/types.ts, the shared types module every layer imports as '../types', got the layer "types.ts" and no rule at all, while the imports into it already resolved to "types". It could import a component or a page unnoticed. A file directly in src/ is now the layer of its name without the extension, for the importer and the import alike. No file changes its count: types.ts imports only @trek/shared today. --- client/CLAUDE.md | 2 +- client/scripts/lib/imports.mjs | 20 ++++++++++++++----- client/scripts/lib/layers.mjs | 3 ++- client/tests/unit/scripts/layer-lint.test.ts | 21 +++++++++++++++++++- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/client/CLAUDE.md b/client/CLAUDE.md index db3c515f9..91ea70f18 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -48,7 +48,7 @@ The offline core is flagship work surrounded by a periphery that ignores it. New - **One data path, no layer skips.** Never import `api/client` (or raw fetch/axios) from a component or modal, and especially never *write* from one. `lint:offline` enforces it: a file under `components/`, `mobile/`, `pages/` or `hooks/` may not import anything from `src/api/` except types. The long tail that already does is listed in `scripts/offline-baseline.json`; the list only shrinks (a file that stops leaves it via `--update`) and is debt, not precedent. A domain that is online-only on purpose (admin, auth) still puts its calls in a store or repo, not in the view. - **Offline-first is the product promise.** New domains get a repo with read-through and offline writes (temp id → optimistic Dexie write → idempotent queue entry). If a domain is deliberately online-only, say so explicitly. -- **Imports go downwards** (`lint:layers`, rules in `scripts/lib/layers.mjs`). A component never imports `pages/` or `mobile/`, a shared hook no view, and stores, repos, sync, api, db and the helpers no view at all. A model or helper both sides need moves down (`utils/`, `hooks/`, `types/`); the counts in `scripts/layers-baseline.json` are debt, not precedent. +- **Imports go downwards** (`lint:layers`, rules in `scripts/lib/layers.mjs`). A component never imports `pages/` or `mobile/`, a shared hook no view, and stores, repos, sync, api, db and the helpers no view at all; a file directly in `src/` is the layer of its name, so `src/types.ts` follows the `types/` rules. A model or helper both sides need moves down (`utils/`, `hooks/`, `types/`); the counts in `scripts/layers-baseline.json` are debt, not precedent. - **No god components or god hooks.** The page pattern is a floor, not a ceiling. Decompose hooks by concern, split renders into memoizable sections, and don't pass huge prop bags with inline-arrow callbacks. - **Prefer React 19 idioms**: `useOptimistic`, `useActionState`/`useFormStatus`, `use()` + Suspense, `useSyncExternalStore` for external subscriptions (`hooks/useNetworkMode` is the reference). - **Optimistic writes must reconcile** — rollback + user-visible toast on failure. No `.catch(() => {})`, no bare `catch {}`. diff --git a/client/scripts/lib/imports.mjs b/client/scripts/lib/imports.mjs index c5d48752e..7a016f5c9 100644 --- a/client/scripts/lib/imports.mjs +++ b/client/scripts/lib/imports.mjs @@ -53,9 +53,19 @@ export function importsOf(source, file) { } /** - * The top directory under src/ a relative import lands in ("components", - * "api"; a file directly in src/ gives its own name), or null for a package - * or a path outside src/. + * The layer of a path from src/: its top directory ("components", "api"), or, + * for a file directly in src/, its name without the extension. src/types.ts + * is then the layer "types" both when it imports and when it is imported as + * '../types', and its rules are those of the src/types/ directory. + */ +export function layerOfPath(fromSrc) { + const [top, ...rest] = fromSrc.split('/'); + return rest.length ? top : top.replace(/(\.d)?\.[cm]?[jt]sx?$/, ''); +} + +/** + * The layer a relative import lands in (see layerOfPath), or null for a + * package or a path outside src/. */ export function layerOf(srcRoot, fromFile, specifier) { if (!specifier.startsWith('.')) return null; @@ -63,7 +73,7 @@ export function layerOf(srcRoot, fromFile, specifier) { .split('\\') .join('/'); if (!target || target.startsWith('..')) return null; - return target.split('/')[0]; + return layerOfPath(target); } /** Every source file under root/src outside the tests, with its key, its layer and its imports resolved to layers. */ @@ -71,7 +81,7 @@ export function sourceGraph(root) { const src = join(root, 'src'); return listFiles(root, ['src'], (key) => /\.tsx?$/.test(key) && !TEST_FILE.test(key)).map((path) => ({ key: toKey(root, path), - layer: toKey(src, path).split('/')[0], + layer: layerOfPath(toKey(src, path)), imports: importsOf(readText(path), path).map((imp) => ({ ...imp, layer: layerOf(src, path, imp.specifier) })), })); } diff --git a/client/scripts/lib/layers.mjs b/client/scripts/lib/layers.mjs index 637258cae..22e5d6f1b 100644 --- a/client/scripts/lib/layers.mjs +++ b/client/scripts/lib/layers.mjs @@ -17,7 +17,8 @@ const VIEW = ['components', 'mobile', 'pages', 'hooks']; * (date helpers) are helpers like utils/. Directories without an entry are * free on purpose: mobile and pages sit at the top, the app root wires * everything, and managed/ is the slot an operator fills with whole screens - * at build time. + * at build time. A file directly in src/ is the layer of its name, so + * src/types.ts falls under types like the src/types/ directory. */ export const RULES = { components: ['pages', 'mobile'], diff --git a/client/tests/unit/scripts/layer-lint.test.ts b/client/tests/unit/scripts/layer-lint.test.ts index ec90696e8..bbf0e63cd 100644 --- a/client/tests/unit/scripts/layer-lint.test.ts +++ b/client/tests/unit/scripts/layer-lint.test.ts @@ -1,7 +1,7 @@ import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { importsOf, layerOf } from '../../../scripts/lib/imports.mjs'; +import { importsOf, layerOf, layerOfPath } from '../../../scripts/lib/imports.mjs'; import { check, RULES, violations } from '../../../scripts/lib/layers.mjs'; import { RatchetError } from '../../../scripts/lib/ratchet.mjs'; import { ratchetTree, type RatchetTree } from '../../helpers/ratchetFixture'; @@ -57,6 +57,16 @@ describe('scripts/lib/imports', () => { expect(layerOf(src, from, 'react')).toBeNull(); expect(layerOf(src, from, '../../../../shared/src')).toBeNull(); }); + + it('IMPORTS-003: a file directly in src/ is the layer of its name without the extension', () => { + expect(layerOfPath('types.ts')).toBe('types'); + expect(layerOfPath('App.tsx')).toBe('App'); + expect(layerOfPath('vite-env.d.ts')).toBe('vite-env'); + expect(layerOfPath('types/tz-lookup.d.ts')).toBe('types'); + expect(layerOfPath('components/Map/MapView.tsx')).toBe('components'); + const src = join('/r', 'src'); + expect(layerOf(src, join(src, 'App.tsx'), './types.ts')).toBe('types'); + }); }); describe('lint:layers', () => { @@ -125,6 +135,15 @@ describe('lint:layers', () => { expect(tree.error.join('\n')).toMatch(/src\/vacay\/yearWindow\.ts: 1 import\(s\) against the layering/); }); + it('LAYERS-008: src/types.ts is held to the rules of types and imports no view', () => { + const files = { + 'src/types.ts': "import type { Props } from './components/Map/MapView'\nimport type { T } from '@trek/shared'\n", + }; + expect(run({}, files)).toBe(1); + expect(tree.error.join('\n')).toMatch(/src\/types\.ts: 1 import\(s\) against the layering/); + expect(tree.error.join('\n')).toMatch(/\(types into components\)/); + }); + it('LAYERS-006: a missing baseline stops the check', () => { tree = ratchetTree({ 'src/a.ts': '' }); expect(() => check({ root: tree.root, baselinePath: tree.path(BASELINE), ...tree.out })).toThrow(RatchetError);