Skip to content

Commit 100c12d

Browse files
DavidBabinecDavidBabinec
authored
refactor(admin): remove circular deps and recover lazy HMR loads (CoreBunch#83)
1 parent 0e0121f commit 100c12d

89 files changed

Lines changed: 1028 additions & 640 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎docs/reference/architecture-tests.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Catalog of every test in `src/__tests__/architecture/`. These are structural gat
66

77
## TL;DR
88

9-
- 89 gate files across structural domains: SQL, JSON columns, migrations, CSS, icons, primitives, page tree, sandbox, agent, router, content storage, boundary validation, module size, AI, auth, error handling, etc.
9+
- 90 gate files across structural domains: SQL, JSON columns, migrations, CSS, icons, primitives, page tree, sandbox, agent, router, content storage, boundary validation, module size, AI, auth, error handling, etc.
1010
- Naming convention: `<topic>.test.ts` (kebab-case) or `<group>-<topic>.test.ts`. A few legacy `task<N>-*` ids remain for live invariants; new gates should use topic names.
1111
- Run them all: `bun test src/__tests__/architecture/`.
1212
- Most are **import / source scans** — they parse the files in scope and assert / reject patterns. Some are unit-style (a small in-test database, a synthesized page tree).
@@ -114,6 +114,7 @@ See [docs/reference/ui-primitives.md](ui-primitives.md).
114114
| Test | What it enforces |
115115
|-----------------------------------------------|----------------------------------------------------------------------------------|
116116
| `canvasFastRefreshBoundaries.test.ts` | `.tsx` files don't mix component + non-component exports (breaks HMR). |
117+
| `no-circular-dependencies.test.ts` | `madge` finds zero tsconfig-aware circular dependencies across `src` and `server`. |
117118
| `canvas-aware-selectors.test.ts` | Canvas-related store selectors are subscribed correctly to canvas-state slices. |
118119
| `admin-router-usage.test.ts` | Internal admin navigation uses `@admin/lib/routing`; raw `/admin` anchors and `react-router-dom` are banned. |
119120
| `framework-typography-spacing.test.ts` | The site framework's typography / spacing tokens compile correctly. |

‎server/repositories/media.ts‎

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,71 +6,15 @@ import {
66
parseVariants,
77
type MediaAssetRow,
88
} from './mediaAssetMapping'
9+
import type { MediaAsset, MediaVariant } from './mediaTypes'
910

1011
// The row ↔ asset mapping unit (column constants, `MediaAssetRow`,
1112
// `mapMediaAssetRow`, and the JSON parsers) lives in `./mediaAssetMapping` so it
1213
// can be shared verbatim with the publisher's render-time prefetch without
1314
// duplication. This module owns the asset domain types (`MediaAsset`,
1415
// `MediaVariant`) and every CRUD query.
1516

16-
export interface MediaVariant {
17-
width: number
18-
height: number
19-
format: 'webp' | 'jpeg' | 'png' | 'avif'
20-
/**
21-
* Public URL the renderer emits (`/uploads/<storage>` for local; an
22-
* absolute URL like `https://cdn.example.com/...` for `'public-url'`
23-
* adapters; `/uploads/<storage>` again for `'signed-redirect'` /
24-
* `'proxy'` adapters because the router resolves them on request).
25-
*/
26-
path: string
27-
sizeBytes: number
28-
/**
29-
* Adapter-internal storage handle. For local-disk this is the basename
30-
* under `uploadsDir`; for S3 it's the bucket key. Used by `dispatchDelete`
31-
* to remove the right bytes when the asset is purged.
32-
*/
33-
storagePath: string
34-
/** Adapter id that wrote this variant; `''` for the built-in local-disk adapter. */
35-
storageAdapterId: string
36-
}
37-
38-
export interface MediaAsset {
39-
id: string
40-
filename: string
41-
mimeType: string
42-
sizeBytes: number
43-
publicPath: string
44-
uploadedByUserId: string | null
45-
createdAt: string
46-
altText: string
47-
caption: string
48-
title: string
49-
tags: string[]
50-
width: number | null
51-
height: number | null
52-
durationMs: number | null
53-
dominantColor: string | null
54-
deletedAt: string | null
55-
replacedAt: string | null
56-
folderIds: string[]
57-
blurHash: string | null
58-
variants: MediaVariant[]
59-
posterPath: string | null
60-
/**
61-
* Id of the storage adapter that wrote this asset. Empty string for the
62-
* built-in local-disk adapter (historical assets keep this default).
63-
* Reads dispatch through THIS field, not the currently-elected adapter,
64-
* so an election swap can't strand existing rows.
65-
*/
66-
storageAdapterId: string
67-
/**
68-
* True when the bytes live outside the host's uploads dir
69-
* (servingMode `'public-url'`). The hard-delete path uses this to choose
70-
* between local `rm` and `adapter.delete()`.
71-
*/
72-
externallyHosted: boolean
73-
}
17+
export type { MediaAsset, MediaVariant } from './mediaTypes'
7418

7519
interface CreateMediaAssetInput {
7620
id: string

‎server/repositories/mediaAssetMapping.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
import { isoDate, isoDateOrNull } from '@core/utils/isoDate'
21-
import type { MediaAsset, MediaVariant } from './media'
21+
import type { MediaAsset, MediaVariant } from './mediaTypes'
2222

2323
/**
2424
* Single source of truth for the hydrated media-asset projection. Spliced into

‎server/repositories/mediaTypes.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* Media repository domain types.
3+
*
4+
* Kept outside the CRUD repository and row mapper so both can share the same
5+
* asset shape without importing each other.
6+
*/
7+
8+
export interface MediaVariant {
9+
width: number
10+
height: number
11+
format: 'webp' | 'jpeg' | 'png' | 'avif'
12+
/**
13+
* Public URL the renderer emits (`/uploads/<storage>` for local; an absolute
14+
* URL for public external storage; local route again for redirect/proxy modes).
15+
*/
16+
path: string
17+
sizeBytes: number
18+
/** Adapter-internal storage handle. */
19+
storagePath: string
20+
/** Adapter id that wrote this variant; `''` for the built-in local adapter. */
21+
storageAdapterId: string
22+
}
23+
24+
export interface MediaAsset {
25+
id: string
26+
filename: string
27+
mimeType: string
28+
sizeBytes: number
29+
publicPath: string
30+
uploadedByUserId: string | null
31+
createdAt: string
32+
altText: string
33+
caption: string
34+
title: string
35+
tags: string[]
36+
width: number | null
37+
height: number | null
38+
durationMs: number | null
39+
dominantColor: string | null
40+
deletedAt: string | null
41+
replacedAt: string | null
42+
folderIds: string[]
43+
blurHash: string | null
44+
variants: MediaVariant[]
45+
posterPath: string | null
46+
/** Empty string for the built-in local-disk adapter. */
47+
storageAdapterId: string
48+
/** True when bytes live outside the host uploads dir. */
49+
externallyHosted: boolean
50+
}

‎src/__tests__/architecture/canvasFastRefreshBoundaries.test.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readFileSync } from 'node:fs'
1+
import { readdirSync, readFileSync, statSync } from 'node:fs'
22
import { join } from 'node:path'
33
import { describe, expect, it } from 'bun:test'
44

@@ -8,6 +8,17 @@ function readSource(path: string): string {
88
return readFileSync(join(SRC_ROOT, path), 'utf8')
99
}
1010

11+
function collectFiles(dir: string): string[] {
12+
const files: string[] = []
13+
for (const entry of readdirSync(dir)) {
14+
const full = join(dir, entry)
15+
const stat = statSync(full)
16+
if (stat.isDirectory()) files.push(...collectFiles(full))
17+
else if (entry.endsWith('Editor.tsx')) files.push(full)
18+
}
19+
return files
20+
}
21+
1122
describe('Canvas Fast Refresh boundaries', () => {
1223
it('keeps component modules free of Fast Refresh suppression comments', () => {
1324
const files = [
@@ -34,4 +45,19 @@ describe('Canvas Fast Refresh boundaries', () => {
3445

3546
expect(source).not.toContain('export function createSandboxSrcDoc')
3647
})
48+
49+
it('keeps base module editors independent of registration barrels', () => {
50+
const editorFiles = collectFiles(join(SRC_ROOT, 'modules/base'))
51+
expect(editorFiles.length).toBeGreaterThan(0)
52+
53+
const offenders: string[] = []
54+
for (const file of editorFiles) {
55+
const source = readFileSync(file, 'utf8')
56+
if (/from ['"]\.\/index['"]/.test(source)) {
57+
offenders.push(file.replace(`${SRC_ROOT}/`, ''))
58+
}
59+
}
60+
61+
expect(offenders).toEqual([])
62+
})
3763
})

‎src/__tests__/architecture/no-circular-dependencies.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { describe, expect, it } from 'bun:test'
2+
import { join } from 'node:path'
3+
4+
const ROOT = join(import.meta.dir, '../../..')
5+
6+
function decode(bytes: Uint8Array): string {
7+
return new TextDecoder().decode(bytes)
8+
}
9+
10+
describe('Circular dependencies', () => {
11+
it('keeps the tsconfig-aware source graph cycle-free', () => {
12+
const result = Bun.spawnSync({
13+
cmd: [
14+
process.execPath,
15+
'x',
16+
'madge',
17+
'--circular',
18+
'--ts-config',
19+
'tsconfig.json',
20+
'--extensions',
21+
'ts,tsx',
22+
'src',
23+
'server',
24+
],
25+
cwd: ROOT,
26+
stdout: 'pipe',
27+
stderr: 'pipe',
28+
})
29+
30+
const output = `${decode(result.stdout)}${decode(result.stderr)}`
31+
if (result.exitCode !== 0) {
32+
throw new Error(
33+
`Circular dependencies found. Run the same command locally for the full graph:\n` +
34+
`bun x madge --circular --ts-config tsconfig.json --extensions ts,tsx src server\n\n` +
35+
output,
36+
)
37+
}
38+
39+
expect(output).toContain('No circular dependency found')
40+
})
41+
})

‎src/__tests__/architecture/site-editor-shell-lazy-body.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ describe('Site editor shell lazy body', () => {
2424
const body = readAdminFile('layouts/AdminCanvasLayout/AdminCanvasEditorBody.tsx')
2525

2626
expect(layout).toContain("import('./AdminCanvasEditorBody')")
27+
expect(layout).toContain('prewarmedLazy<AdminCanvasEditorBodyProps>')
28+
expect(layout).toContain('<LazyChunkBoundary')
29+
expect(layout).toContain('onReset={AdminCanvasEditorBody.reset}')
2730
expect(layout).toContain('scheduleAfterFirstPaint')
2831
expect(layout).not.toContain("@dnd-kit/core")
2932
expect(layout).not.toContain("@admin/pages/site/canvas")
@@ -60,6 +63,7 @@ describe('Site editor shell lazy body', () => {
6063
// shell must still paint canvas-shaped skeleton frames; otherwise cold Site
6164
// loads show only the toolbar over an empty black workspace.
6265
expect(layout).toContain('<AdminCanvasEditorBodyLoading />')
66+
expect(layout).toContain('fallback={<AdminCanvasEditorBodyLoading />}')
6367
expect(layout).toContain('@admin/shared/CanvasFrameSkeleton')
6468
expect(layout).toContain('<CanvasFrameSkeletonFrame')
6569
expect(layout).not.toContain('<span>Loading editor</span>')

‎src/admin/layouts/AdminCanvasLayout/AdminCanvasLayout.tsx‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ import {
5454
CanvasFrameSkeletonFrame,
5555
DEFAULT_CANVAS_FRAME_SKELETON_BREAKPOINTS,
5656
} from '@admin/shared/CanvasFrameSkeleton'
57+
import { LazyChunkBoundary } from '@admin/lib/LazyChunkBoundary'
58+
import { prewarmedLazy } from '@admin/lib/prewarmedLazy'
5759
import styles from './AdminCanvasLayout.module.css'
5860
import { lazy, Suspense, useEffect, useState } from 'react'
5961
import { useCurrentAdminUser } from '@admin/sessionContext'
@@ -67,8 +69,16 @@ import {
6769
import { EditorPermissionsProvider } from '@site/EditorPermissionsProvider'
6870
import type { EditorPermissions } from '@site/editorPermissionsContext'
6971

70-
const AdminCanvasEditorBody = lazy(() =>
71-
import('./AdminCanvasEditorBody').then((m) => ({ default: m.AdminCanvasEditorBody })),
72+
interface AdminCanvasEditorBodyProps {
73+
canEditDraftSite: boolean
74+
canSaveSite: boolean
75+
loadError: string | null
76+
}
77+
78+
const AdminCanvasEditorBody = prewarmedLazy<AdminCanvasEditorBodyProps>(
79+
() =>
80+
import('./AdminCanvasEditorBody').then((m) => ({ default: m.AdminCanvasEditorBody })),
81+
{ displayName: 'AdminCanvasEditorBody' },
7282
)
7383

7484
// SettingsModal is heavy (~37 KB raw) and closed 99% of the time. lazy()
@@ -216,13 +226,18 @@ export function AdminCanvasLayout() {
216226
/>
217227

218228
{loadEditorBody ? (
219-
<Suspense fallback={<AdminCanvasEditorBodyLoading />}>
229+
<LazyChunkBoundary
230+
location="site-editor-body"
231+
fallback={<AdminCanvasEditorBodyLoading />}
232+
resetKeys={[site?.id ?? null]}
233+
onReset={AdminCanvasEditorBody.reset}
234+
>
220235
<AdminCanvasEditorBody
221236
canEditDraftSite={canEditDraftSite}
222237
canSaveSite={canSaveSite}
223238
loadError={loadError}
224239
/>
225-
</Suspense>
240+
</LazyChunkBoundary>
226241
) : (
227242
<AdminCanvasEditorBodyLoading />
228243
)}

‎src/admin/lib/LazyChunkBoundary.module.css‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
.chunkFailure {
2+
box-sizing: border-box;
3+
display: grid;
4+
flex: 1 1 auto;
5+
width: 100%;
6+
min-height: 260px;
7+
min-width: 0;
8+
place-items: center;
9+
padding: 32px;
10+
background: var(--editor-bg);
11+
color: var(--editor-text);
12+
}
13+
14+
.panel {
15+
display: flex;
16+
width: min(460px, 100%);
17+
flex-direction: column;
18+
gap: 12px;
19+
padding: 16px;
20+
background: var(--panel-bg);
21+
border: 1px solid var(--panel-border);
22+
border-radius: var(--editor-radius);
23+
box-shadow: var(--panel-shadow);
24+
}
25+
26+
.title {
27+
margin: 0;
28+
color: var(--editor-text-bright);
29+
font-size: 14px;
30+
font-weight: 650;
31+
line-height: 1.3;
32+
}
33+
34+
.message {
35+
margin: 0;
36+
color: var(--editor-text-secondary);
37+
font-size: 12px;
38+
line-height: 1.5;
39+
overflow-wrap: anywhere;
40+
}
41+
42+
.actions {
43+
display: flex;
44+
align-items: center;
45+
gap: 8px;
46+
}

0 commit comments

Comments
 (0)