Skip to content

Commit 6d868e1

Browse files
os-zhuangclaude
andauthored
refactor(layout)!: delete PageNodeRenderer, the unregistered page-node renderer (#3225)
`@object-ui/layout`'s `PageNodeRenderer` (`./Page`, named `Page` before objectui#3161 batch 7) was registered under no component key and imported by nothing — a whole-repo grep finds zero call sites. It reached consumers only through `export * from './Page'`, and `registerLayout()` already carried a note telling the next reader NOT to register it: the `page` key belongs to `@object-ui/components`'s `PageRenderer`, which is the renderer that supports page types, named regions and `PageVariablesProvider`. Removed under ADR-0049 (enforce-or-remove). Wiring it up instead would mean two renderers for one key — a fork nothing is asking for — while leaving it in place kept telling every reader that `@object-ui/layout` is where page rendering lives. Its props were also `{ schema: PageNodeSchema; … } & any`, and an intersection with `any` absorbs the whole type, so the signature asserted nothing beyond "there is a schema" (objectui#3221 mechanism, fourth spelling). - delete `src/Page.tsx` and its barrel re-export; the `registerLayout` note now records why no second `page` renderer belongs here - add `page-node-renderer-stays-deleted.test.ts`: a source-level tombstone so re-adding the export is a red test naming the decision, not a one-line gap-fill (verified red by reintroducing the export) - keep the batch-7 type pins, including the repo's only pin of `PageNodeSchema['type'] === 'page'` — that is the wire key `PageRenderer` answers to, and it outlives the component it was written beside - `tsconfig.test.json` names `types: ["node"]` for the tombstone's source scan, matching `packages/types/tsconfig.test.json`; it stays out of `tsconfig.json` so package source still cannot compile against Node APIs - README stopped documenting `<Page>` (an example already stale since the batch-7 rename) and points at `PageRenderer` instead Fixes #3223 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4a51e77 commit 6d868e1

8 files changed

Lines changed: 175 additions & 86 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@object-ui/layout": major
3+
---
4+
5+
Remove `PageNodeRenderer`, the dead page-node renderer (objectui#3223, ADR-0049
6+
enforce-or-remove).
7+
8+
**Removed:** the `PageNodeRenderer` export and its `./Page` module. It was
9+
registered under no component key and imported by nothing — a whole-repo grep
10+
found zero call sites — so it reached consumers only through
11+
`export * from './Page'` in the package barrel. `registerLayout()` was already
12+
saying so in a note that told the next reader *not* to register it. Its props
13+
were also `{ schema: PageNodeSchema; … } & any`, and an intersection with `any`
14+
absorbs the whole type, so the signature asserted nothing beyond "there is a
15+
schema".
16+
17+
**Migration:** there is nothing to re-point in a working app — an unregistered
18+
renderer had no call site to migrate. If you imported the symbol directly:
19+
20+
```diff
21+
-import { PageNodeRenderer } from '@object-ui/layout';
22+
+import { PageRenderer } from '@object-ui/components';
23+
```
24+
25+
`PageRenderer` in `@object-ui/components` is, and remains, the renderer for the
26+
`page` component key. It is the one that supports page types
27+
(record/home/app/utility), named regions and `PageVariablesProvider` — the
28+
deleted one rendered a header plus children and nothing else. Schema-driven
29+
consumers are unaffected: a `{ type: 'page' }` node has always resolved through
30+
the registry to `PageRenderer`, never to this export.
31+
32+
Also note: this supersedes the `Page``PageNodeRenderer` rename shipped for
33+
this package in the batch 7 symbol burn-down — the renamed symbol is gone rather
34+
than renamed again. `PageHeaderProps``PageHeaderComponentProps` from that same
35+
batch is unaffected.

packages/layout/README.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,23 @@ import { AppShell } from '@object-ui/layout';
3838
</AppShell>
3939
```
4040

41-
### Page
41+
### PageHeader
4242

43-
Standard page layout with optional header section.
43+
Page title block with an optional description, used at the top of a page's
44+
content area.
4445

4546
```typescript
46-
import { Page, PageHeader } from '@object-ui/layout';
47+
import { PageHeader } from '@object-ui/layout';
4748

48-
<Page>
49-
<PageHeader title="Dashboard" description="View your metrics" />
50-
<div>Page Content</div>
51-
</Page>
49+
<PageHeader title="Dashboard" description="View your metrics" />
5250
```
5351

52+
> **Rendering a whole `page` node?** That belongs to `PageRenderer` in
53+
> `@object-ui/components`, which is what the `page` component key resolves to —
54+
> it handles page types (record/home/app/utility), named regions and page
55+
> variables. This package deliberately does not register or export a second
56+
> renderer for that key (objectui#3223).
57+
5458
### SidebarNav
5559

5660
Navigation sidebar component with React Router integration.

packages/layout/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
"vite-plugin-dts": "^5.0.3",
4646
"vitest": "^4.1.10"
4747
},
48-
"description": "Layout components for Object UI — AppShell, Page, SidebarNav and responsive containers with React Router integration",
48+
"description": "Layout components for Object UI — AppShell, PageHeader, SidebarNav and responsive containers with React Router integration",
4949
"keywords": [
5050
"objectui",
5151
"sdui",

packages/layout/src/Page.tsx

Lines changed: 0 additions & 60 deletions
This file was deleted.
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `@object-ui/layout` has no page-node renderer — the tombstone for
11+
* objectui#3223.
12+
*
13+
* The `page` key belongs to `@object-ui/components`'s `PageRenderer`: it is the
14+
* one that understands page types (record/home/app/utility), named regions and
15+
* `PageVariablesProvider`. This package carried a second renderer for the same
16+
* node (`Page`, renamed `PageNodeRenderer` in objectui#3161 / objectstack#4115
17+
* batch 7) that was registered nowhere and imported by nothing — reachable only
18+
* as a public export. Deleted under ADR-0049 (enforce-or-remove).
19+
*
20+
* Why a test and not just the deletion: what made the dead renderer expensive
21+
* was never its behaviour — it had none, nothing called it — but the claim its
22+
* mere presence made. A reader (or an AI author) who greps `PageNodeRenderer`
23+
* and finds it exported from `@object-ui/layout` reasonably concludes that this
24+
* package is where page rendering lives, and wires a second implementation of a
25+
* key that already has an owner. Re-adding the export is a one-line change and
26+
* looks like filling a gap; this file is what turns it into a red test naming
27+
* the decision to reverse first.
28+
*
29+
* Scope, stated honestly: this is a SOURCE-level ban inside this package, not a
30+
* proof about the composed registry. It cannot tell you which renderer wins at
31+
* runtime — `packages/components`' own registration tests own that question.
32+
* What it pins is that `@object-ui/layout` does not re-enter the race.
33+
*/
34+
35+
import { describe, it, expect } from 'vitest';
36+
import { existsSync, readFileSync, readdirSync } from 'node:fs';
37+
import { join, resolve } from 'node:path';
38+
39+
/** `packages/layout/src` — one level up from `src/__tests__`. */
40+
const SRC = resolve(__dirname, '..');
41+
42+
/** Source files that legitimately discuss the removal (this file, and the
43+
* barrel's note explaining why `page` is not registered here). */
44+
const MAY_MENTION = new Set([
45+
'__tests__/page-node-renderer-stays-deleted.test.ts',
46+
'__tests__/spec-symbol-batch7.test.ts',
47+
'index.ts',
48+
]);
49+
50+
/** Every `.ts`/`.tsx` under `src/`, relative to `src/`. */
51+
function sourceFiles(dir = SRC, prefix = ''): string[] {
52+
return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
53+
const rel = prefix ? `${prefix}/${entry.name}` : entry.name;
54+
if (entry.isDirectory()) return sourceFiles(join(dir, entry.name), rel);
55+
return /\.tsx?$/.test(entry.name) ? [rel] : [];
56+
});
57+
}
58+
59+
describe('the layout page-node renderer stays deleted (objectui#3223)', () => {
60+
it('has no `./Page` module', () => {
61+
expect(existsSync(join(SRC, 'Page.tsx'))).toBe(false);
62+
expect(existsSync(join(SRC, 'Page.ts'))).toBe(false);
63+
});
64+
65+
it('does not re-export it from the package barrel', () => {
66+
const barrel = readFileSync(join(SRC, 'index.ts'), 'utf8');
67+
expect(barrel).not.toMatch(/export\s+\*\s+from\s+['"]\.\/Page['"]/);
68+
// …nor under a named re-export. The barrel's prose may still explain the
69+
// removal (it does); only an `export { … }` naming it is banned.
70+
expect(barrel).not.toMatch(/export\s*(type\s*)?\{[^}]*\bPageNodeRenderer\b/);
71+
});
72+
73+
it('does not register the `page` key — that owner is @object-ui/components', () => {
74+
const barrel = readFileSync(join(SRC, 'index.ts'), 'utf8');
75+
// `page:card` / `page-header` are this package's own keys and stay.
76+
expect(barrel).not.toMatch(/ComponentRegistry\.register\(\s*['"]page['"]/);
77+
});
78+
79+
it('leaves no source file naming the removed component', () => {
80+
const offenders = sourceFiles()
81+
.filter((file) => !MAY_MENTION.has(file))
82+
.filter((file) => /\bPageNodeRenderer\b/.test(readFileSync(join(SRC, file), 'utf8')));
83+
84+
expect(offenders).toEqual([]);
85+
});
86+
});

packages/layout/src/__tests__/spec-symbol-batch7.test.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,29 @@
1010
* `@object-ui/layout` ↔ `@objectstack/spec` symbol-collision guards
1111
* (objectui#3161, objectstack#4115 ledger batch 7).
1212
*
13-
* Both symbols here were the RENDERED layer wearing the AUTHORED layer's name:
13+
* Both symbols batch 7 touched here were the RENDERED layer wearing the
14+
* AUTHORED layer's name. They ended differently, and the difference is the
15+
* point:
1416
*
1517
* - `PageHeaderProps` → `PageHeaderComponentProps`, the name
1618
* `@object-ui/app-shell` already settled on for its own header props
1719
* (objectui#3169). Reused deliberately: one concept, one name, even across
18-
* two packages that each draw their own header.
19-
* - `Page` → `PageNodeRenderer`. The spec's `Page` is the authored page
20-
* DOCUMENT; this renders `@object-ui/types`'s `PageNodeSchema`, the
21-
* schema-renderer NODE that objectui#3074 had already renamed off
22-
* `PageSchema` for exactly this reason.
20+
* two packages that each draw their own header. Still exported, still
21+
* guarded below.
22+
* - `Page` → `PageNodeRenderer` → **deleted** (objectui#3223, ADR-0049
23+
* enforce-or-remove). The rename was correct and did not go far enough: the
24+
* renderer was registered nowhere and called from nowhere — the `page` key
25+
* belongs to `@object-ui/components`'s `PageRenderer`, and `registerLayout`
26+
* actively discourages re-registering it — so all the export did was tell
27+
* the next reader that `@object-ui/layout` is where page rendering lives.
28+
*
29+
* What survives that deletion is the LAYER SPLIT it was renamed for, pinned
30+
* below: the spec's `Page` is the authored page DOCUMENT, `@object-ui/types`'s
31+
* `PageNodeSchema` is the SDUI NODE discriminated by `type: 'page'`, and the
32+
* two are not interchangeable. Those pins are load-bearing beyond the deleted
33+
* component — `type: 'page'` is the wire key `PageRenderer` is registered
34+
* under, and this is the only place in the repo that pins it — so they are
35+
* kept here rather than deleted along with their former subject.
2336
*
2437
* Each `import type` below is load-bearing: if the spec retires one of these
2538
* names, this file stops compiling, and the rename's justification is up for
@@ -37,13 +50,11 @@ import type { PageNodeSchema } from '@object-ui/types';
3750

3851
import { PageHeader } from '../PageHeader';
3952
import type { PageHeaderComponentProps } from '../PageHeader';
40-
import { PageNodeRenderer } from '../Page';
4153

42-
describe('the renamed exports are still the components they were', () => {
43-
it('exports both renderers under their new names', () => {
54+
describe('the renamed export is still the component it was', () => {
55+
it('exports the renderer under its new name', () => {
4456
expect(typeof PageHeader).toBe('function');
45-
expect(typeof PageNodeRenderer).toBe('function');
46-
expect(PageNodeRenderer.name).toBe('PageNodeRenderer');
57+
expect(PageHeader.name).toBe('PageHeader');
4758
});
4859
});
4960

@@ -75,8 +86,12 @@ describe('the spec names still mean the authored layer', () => {
7586
type _RenderedHasSchemaNode = Assert<HasKey<PageHeaderComponentProps, 'schema'>>;
7687

7788
// `Page` in the spec is the page DOCUMENT (`name` + `label` identify it);
78-
// what this package renders is the SDUI node (`type: 'page'`). Two layers,
79-
// and now two names.
89+
// the SDUI node is `PageNodeSchema`, tagged `type: 'page'`. Two layers, two
90+
// names — pinned here even though objectui#3223 deleted this package's
91+
// page-node renderer, because `type: 'page'` is the registry key
92+
// `@object-ui/components`'s `PageRenderer` answers to, and nowhere else in
93+
// the repo pins it. Collapse these two types back together and the next
94+
// renderer named after the wrong layer compiles clean.
8095
type _DocumentIsNotNode = Assert<Equal<Equal<SpecPage, PageNodeSchema>, false>>;
8196
type _DocumentIsIdentified = Assert<HasKey<SpecPage, 'name'>>;
8297
type _NodeIsATaggedNode = Assert<Equal<PageNodeSchema['type'], 'page'>>;

packages/layout/src/index.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@
66
import { ComponentRegistry } from '@object-ui/core';
77
import { PageHeader } from './PageHeader';
88
import { AppShell } from './AppShell';
9-
// NOTE: `PageNodeRenderer` (./Page) is deliberately NOT imported here — it is
10-
// re-exported below but never registered; the `page` key belongs to
11-
// `@object-ui/components`'s `PageRenderer`. See the registration note at the
12-
// bottom of `registerLayout`.
139
import { PageCard } from './PageCard';
1410
import { SidebarNav } from './SidebarNav';
1511
import { ResponsiveGrid } from './ResponsiveGrid';
@@ -18,7 +14,6 @@ import { AppSchemaRenderer } from './AppSchemaRenderer';
1814

1915
export * from './PageHeader';
2016
export * from './AppShell';
21-
export * from './Page';
2217
export * from './PageCard';
2318
export * from './SidebarNav';
2419
export * from './ResponsiveGrid';
@@ -97,6 +92,14 @@ export function registerLayout() {
9792
// NOTE: 'page' registration is handled by @object-ui/components PageRenderer.
9893
// That renderer supports page types (record/home/app/utility), named regions,
9994
// and PageVariablesProvider. Do NOT re-register 'page' here to avoid conflicts.
95+
//
96+
// This package used to ALSO export a `page`-node renderer (`PageNodeRenderer`,
97+
// `./Page`) that this note kept unregistered — so it had no call site and
98+
// never ran, while still advertising itself from the public API as if
99+
// `@object-ui/layout` were where page rendering lives. Deleted in
100+
// objectui#3223 under ADR-0049 (enforce-or-remove): one key, one renderer. If
101+
// the `page` node needs something layout owns, add it to the components
102+
// renderer — do not reintroduce a second one here.
100103
}
101104

102105
// Keep backward compatibility for now if called directly

packages/layout/tsconfig.test.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66
"extends": "../../tsconfig.json",
77
"compilerOptions": {
88
"noEmit": true,
9+
// `page-node-renderer-stays-deleted.test.ts` reads this package's own
10+
// sources off disk to prove the renderer objectui#3223 removed has not been
11+
// re-exported. Same reason `packages/types/tsconfig.test.json` names it, and
12+
// same reason it stays OUT of `tsconfig.json`: package SOURCE ships to
13+
// browsers and must not compile against Node APIs.
14+
"types": ["node"],
915
// Drop the root tsconfig's source-tree `paths` so `@object-ui/*` resolves
1016
// through the workspace dependency's built `.d.ts` instead of pulling
1117
// sibling sources in as program inputs (TS6059).

0 commit comments

Comments
 (0)