Skip to content

Commit 110a567

Browse files
os-zhuangclaude
andcommitted
Merge origin/main into claude/chart-view-label-icon-385ace
Conflict was structural, not semantic: #3039 (issue #2918, the `layout` nav-type gap) and this branch (issue #2916) each removed a different entry from the same three-line `DEBT` object in `scripts/check-type-check-coverage.mjs`. Both removals are correct and independent, so the resolution keeps neither — `DEBT` is now empty, which the guard reports as `0 known-broken`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 parents c7571df + b5b97e2 commit 110a567

49 files changed

Lines changed: 945 additions & 109 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@object-ui/components": patch
3+
"@object-ui/console": patch
4+
---
5+
6+
fix(registry): prefix every namespaced key exactly once, in every namespace
7+
8+
objectui#3023 fixed eleven `record:*` blocks registered as
9+
`register('record:x', …, { namespace: 'record' })` — an already-prefixed name
10+
handed to a registry that prefixes it again, landing the block at
11+
`record:record:x` — and guarded that namespace alone. Twenty-two more were
12+
sitting in `action:` (5), `element:` (10) and `page:` (7), two of them
13+
(`page:header`, `element:divider`) curated public blocks.
14+
15+
Checking one namespace is exactly what let them keep sitting there, so the
16+
guard now asks the whole registry rather than a prefix of it.
17+
18+
Same fix as before: register the bare name and let `namespace` do the
19+
prefixing, with `skipFallback: true` so the fallback does not claim that bare
20+
name globally. It would otherwise take over `header`, `footer`, `sidebar`,
21+
`tabs`, `card`, `accordion`, `section`, `text`, `image`, `button`, `icon`
22+
every one of which belongs to `ui:`. All 22 stay reachable exactly as
23+
`<namespace>:<name>`; the registry goes 522 keys to 500, and the contract is
24+
unchanged at 42/42.
25+
26+
Found while probing why six curated Tier B primitives report no `inputs`. They
27+
do declare them — `vitest.setup.dom.tsx` registers simplified `text` / `image` /
28+
`html` / `grid` stubs that shadow the real registrations inside the test
29+
environment only. That shadowing is a separate question, left alone here; the
30+
doubled keys it turned up are not test-environment artifacts.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@object-ui/types": minor
3+
"@object-ui/layout": patch
4+
"@object-ui/plugin-designer": patch
5+
"@object-ui/i18n": patch
6+
---
7+
8+
fix(types,layout): nav item type `component` joins `NavigationItemType` and its zod enum — objectui#2918
9+
10+
The renderers have carried a full `type: 'component'` implementation (Phase 3b:
11+
`componentRef` colon-split to `/component/<ns>/<name>`, `params` serialised as
12+
querystring, `metadata:*` special-cases) — but the vocabulary never gained the
13+
member, and `@objectstack/spec` has had `ComponentNavItem` all along. The zod
14+
enum was the part that bit: `NavigationItemTypeSchema` rejected
15+
`type: 'component'` at validation time, so authors could not declare one and
16+
the renderer half was unreachable — dead on arrival rather than dead code.
17+
18+
- `NavigationItemType` and `NavigationItemTypeSchema` gain `'component'`;
19+
`NavigationItem` gains the fields the renderer consumes, `componentRef` and
20+
`params` (also used by `type: 'page'`), mirroring spec's `ComponentNavItem`
21+
declared in zod too, so parse no longer strips them.
22+
- The `(item as any).componentRef` / `params` casts in `NavigationRenderer`
23+
and `AppSchemaRenderer` become typed access.
24+
- `NavigationDesigner`'s exhaustive type-meta map gains a `component` badge
25+
(new `appDesigner.navTypeComponent` key in all 10 locales).
26+
- `@object-ui/layout` gains `type-check` (src + tests) with the #2915 `paths`
27+
override; its DEBT entry in `check-type-check-coverage.mjs` is deleted.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
'@object-ui/auth': patch
3+
---
4+
5+
refactor(auth): derive the org-role vocabulary from `@objectstack/spec` instead of mirroring it
6+
7+
`org-roles.ts` restated the four membership-role names that `@objectstack/spec`
8+
owns as `BUILTIN_MEMBERSHIP_ROLES`. That was a mirror for packaging reasons
9+
only: this package took no dependency on the spec, and no published spec
10+
carried the constants. Both blockers are gone — `@objectstack/spec@17.0.0-rc.0`
11+
ships ADR-0108's closed vocabulary and the workspace already pins
12+
`^17.0.0-rc.0` — so the four `ORG_ROLE_*` constants are now re-exports,
13+
`OrgRole` is `BuiltinMembershipRole`, and `ORG_ROLES` is
14+
`[...BUILTIN_MEMBERSHIP_ROLES]`. The list cannot drift from what the server's
15+
enforced `select` accepts, by construction.
16+
17+
Deliberately still local: `ORG_ROLE_LABELS` and the grade ladder
18+
(`orgRoleGrade` / `invitableOrgRoles` / `assignableOrgRoles`). They are console
19+
concerns — i18n keys and screen-narrowing rules — and folding them into the
20+
name list would be the modeling error ADR-0108 D4 warns about: *what names
21+
exist* is a list; *which names mean authority* and *how a name projects* are
22+
rules that belong next to what they govern.
23+
24+
The #2907 drift guard (`is EXACTLY the framework four`) is dropped — a derived
25+
list cannot drift, and asserting a re-export against a literal is noise. No
26+
behaviour changes: the four names, their display order, and their labels are
27+
exactly what they already were.

.claude/launch.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@
2020
],
2121
"port": 5181
2222
},
23+
{
24+
"name": "grid-bulk-demo",
25+
"runtimeExecutable": "pnpm",
26+
"runtimeArgs": [
27+
"--dir",
28+
"packages/plugin-grid",
29+
"exec",
30+
"vite",
31+
"demo",
32+
"--port",
33+
"5198",
34+
"--strictPort"
35+
],
36+
"port": 5198,
37+
"url": "http://localhost:5198"
38+
},
2339
{
2440
"name": "gantt-demo",
2541
"runtimeExecutable": "pnpm",

apps/console/src/__tests__/public-contract.test.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -202,14 +202,7 @@ describe('PUBLIC_BLOCKS ↔ console coverage (reverse direction)', () => {
202202
});
203203

204204
it('registers each record:* block under one key, prefixed once', () => {
205-
// `register('record:x', …, { namespace: 'record' })` prefixes an
206-
// already-prefixed name: the block lands at `record:record:x` and stays
207-
// reachable only through the un-namespaced fallback, which happens to spell
208-
// `record:x`. Eleven blocks were registered that way. The contract hid it —
209-
// `getPublicConfigs()` rewrites `type` to the curated tag — so nothing
210-
// failed while the registry carried a phantom key per block.
211205
const keys = shippedRecordBlocks();
212-
expect(keys.filter((k) => k.startsWith(`${NS}:${NS}:`))).toEqual([]);
213206
expect(keys.every((k) => k.startsWith(`${NS}:`))).toBe(true);
214207
});
215208

@@ -224,6 +217,27 @@ describe('PUBLIC_BLOCKS ↔ console coverage (reverse direction)', () => {
224217
}
225218
});
226219

220+
it('prefixes every namespaced key exactly once, in every namespace', () => {
221+
// `register('page:header', …, { namespace: 'page' })` hands an
222+
// already-prefixed name to a registry that prefixes it again: the block
223+
// lands at `page:page:header` and stays reachable only through the
224+
// un-namespaced fallback, which happens to spell `page:header`. Nothing
225+
// fails — `getPublicConfigs()` rewrites `type` to the curated tag — so the
226+
// registry quietly carries a phantom key per block.
227+
//
228+
// objectui#3023 fixed the eleven in `record:` and guarded that namespace
229+
// alone. Twenty-two more were sitting in `action:`, `element:` and `page:`,
230+
// two of them (`page:header`, `element:divider`) curated public blocks.
231+
// Checking one namespace is what let them keep sitting there, so this asks
232+
// the whole registry.
233+
const doubled = ComponentRegistry.getKnownTypes().filter((k) => {
234+
const ns = ComponentRegistry.getMeta(k)?.namespace;
235+
return !!ns && k.startsWith(`${ns}:${ns}:`);
236+
});
237+
238+
expect(doubled).toEqual([]);
239+
});
240+
227241
it('keeps the chatter alias identical to the block it aliases', () => {
228242
// `record:chatter` is excluded because it duplicates `record:discussion`,
229243
// not because it is lesser. The moment the two configuration surfaces
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* objectui#3025 ratchet (AGENTS.md Commandment #6 — "No `any`"). Same shape as
6+
* the ADR-0054 and #2269 ratchets: runs in the gating `pnpm test` job and fails
7+
* if the anti-pattern reappears.
8+
*
9+
* The bug this guards. Two call sites carried
10+
* `const PanelGroup = ResizablePanelGroup as React.FC<any>`. The cast erased
11+
* the component's props to `any`, so when react-resizable-panels v4 renamed
12+
* `PanelGroup`'s `direction` prop to `orientation`, the stale `direction` kept
13+
* type-checking against nothing — for a whole major version. It silently became
14+
* a dead prop that React forwarded to the DOM as a stray attribute. Removing
15+
* the casts is what turned it back into a hard `TS2322`.
16+
*
17+
* Both comments blamed the toolchain (vite-plugin-dts "not resolving the prop
18+
* type correctly"; a prop that "does not always narrow cleanly in our TS
19+
* config"). Neither was true. That is the real hazard here: a cast like this
20+
* reads as a workaround for a tooling quirk while actually disabling the check
21+
* that would have caught a breaking upstream rename.
22+
*
23+
* If this fails: do not widen a component's props to `any` to make an error go
24+
* away. Read the dependency's current prop types — the prop was probably
25+
* renamed or removed. If a genuinely-invalid prop must be passed, type the
26+
* escape hatch narrowly (`as ComponentProps<typeof X> & { extra?: T }`), never
27+
* with a blanket `any`.
28+
*
29+
* SCOPE — repo-wide over `packages/<pkg>/src` and `apps/<app>/src`, production
30+
* sources only. Test files are excluded, both to match the existing ratchets
31+
* and because this file necessarily contains the pattern in its own regex.
32+
*/
33+
34+
import { describe, it, expect } from 'vitest';
35+
import { readdirSync, readFileSync, statSync } from 'node:fs';
36+
import path from 'node:path';
37+
import { fileURLToPath } from 'node:url';
38+
39+
const here = path.dirname(fileURLToPath(import.meta.url));
40+
// packages/app-shell/src -> repo root
41+
const repoRoot = path.resolve(here, '../../..');
42+
43+
/**
44+
* Banned: casting a value to a component type parameterised with `any`.
45+
*
46+
* Covers the three spellings that erase props identically — `FC`,
47+
* `FunctionComponent`, `ComponentType` — with or without the `React.`
48+
* qualifier, and tolerant of whitespace inside the type arguments.
49+
*/
50+
const COMPONENT_ANY_CAST =
51+
/\bas\s+(?:React\.)?(?:FC|FunctionComponent|ComponentType)\s*<\s*any\s*>/;
52+
53+
function collectSourceFiles(): string[] {
54+
const out: string[] = [];
55+
const walk = (dir: string) => {
56+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
57+
const name = entry.name;
58+
if (name === 'node_modules' || name === 'dist' || name.startsWith('.wt-') || name === '.next') {
59+
continue;
60+
}
61+
const full = path.join(dir, name);
62+
if (entry.isDirectory()) {
63+
walk(full);
64+
} else if (/\.tsx?$/.test(name) && !/\.(test|spec)\.tsx?$/.test(name)) {
65+
out.push(full);
66+
}
67+
}
68+
};
69+
for (const group of ['packages', 'apps']) {
70+
const groupDir = path.join(repoRoot, group);
71+
let pkgs: string[];
72+
try {
73+
pkgs = readdirSync(groupDir);
74+
} catch {
75+
continue;
76+
}
77+
for (const pkg of pkgs) {
78+
const srcDir = path.join(groupDir, pkg, 'src');
79+
try {
80+
if (statSync(srcDir).isDirectory()) walk(srcDir);
81+
} catch {
82+
/* package has no src/ */
83+
}
84+
}
85+
}
86+
return out;
87+
}
88+
89+
describe('objectui#3025 — no component-props-to-any cast ratchet', () => {
90+
it('finds the repo sources (guards against a broken scan path)', () => {
91+
// The repo has thousands of source files; if this collapses, the scan
92+
// globs have gone stale and the ratchet would silently pass on nothing.
93+
expect(collectSourceFiles().length).toBeGreaterThan(500);
94+
});
95+
96+
it('detects the pattern it is meant to ban (guards against a dead regex)', () => {
97+
// A ratchet whose regex silently stops matching is worse than no ratchet,
98+
// so pin it against the exact casts #3025 removed, plus spelling variants.
99+
for (const sample of [
100+
'const PanelGroup = ResizablePanelGroup as React.FC<any>;',
101+
'const X = Y as FC<any>',
102+
'const X = Y as React.ComponentType< any >',
103+
'const X = Y as FunctionComponent<any>',
104+
]) {
105+
expect(COMPONENT_ANY_CAST.test(sample), sample).toBe(true);
106+
}
107+
// ...and must not fire on legitimate neighbours.
108+
for (const ok of [
109+
'const x = y as Record<string, any>',
110+
'const x = y as ComponentProps<typeof Button>',
111+
'const C: React.FC<Props> = (p) => null',
112+
]) {
113+
expect(COMPONENT_ANY_CAST.test(ok), ok).toBe(false);
114+
}
115+
});
116+
117+
it('has zero component-props-to-any casts in production sources', () => {
118+
const offenders: string[] = [];
119+
for (const file of collectSourceFiles()) {
120+
const src = readFileSync(file, 'utf8');
121+
for (const line of src.split('\n')) {
122+
if (COMPONENT_ANY_CAST.test(line)) {
123+
offenders.push(`${path.relative(repoRoot, file)} :: ${line.trim()}`);
124+
}
125+
}
126+
}
127+
// If this fails: you widened a component's props to `any`. That disables
128+
// the check that catches renamed/removed props across a dependency major —
129+
// exactly how #3025's dead `direction` prop survived. Read the current
130+
// prop types instead, or type the escape hatch narrowly. Do not allowlist.
131+
expect(offenders).toEqual([]);
132+
});
133+
});

packages/auth/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
},
3939
"dependencies": {
4040
"@object-ui/types": "workspace:*",
41+
"@objectstack/spec": "^17.0.0-rc.0",
4142
"better-auth": "^1.6.25"
4243
},
4344
"devDependencies": {

packages/auth/src/__tests__/org-roles.test.ts

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,16 @@ import {
2929
} from '../org-roles';
3030

3131
describe('vocabulary', () => {
32-
it('carries the delegated issuer grade the framework registers', () => {
32+
it('offers the spec vocabulary WHOLE — including the delegated issuer grade', () => {
33+
// The names themselves derive from `@objectstack/spec`
34+
// (`BUILTIN_MEMBERSHIP_ROLES`), so there is no drift to guard against any
35+
// more — #2907's exact-list assertion is gone with the mirror. What this
36+
// pins is a console decision: `ORG_ROLES` is the spec list UNFILTERED.
37+
// Hiding a grade from the screens belongs in the narrowing helpers below,
38+
// never in the vocabulary.
3339
expect(ORG_ROLES).toContain(ORG_ROLE_DELEGATED_ADMIN);
3440
});
3541

36-
it('is EXACTLY the framework four — the closed vocabulary, mirrored', () => {
37-
// [framework ADR-0108 / objectstack#3723] `sys_member.role` is a closed,
38-
// framework-owned list; an app can no longer register a fifth name. This
39-
// is the drift guard until `@object-ui/auth` can import
40-
// `BUILTIN_MEMBERSHIP_ROLES` from `@objectstack/spec` (see org-roles.ts).
41-
//
42-
// Order matters: it is the display order both screens list, and it matches
43-
// `BUILTIN_MEMBERSHIP_ROLE_OPTIONS` server-side.
44-
//
45-
// If this fails because the framework list changed, mirror the change —
46-
// do NOT add a name the server does not offer. A value outside this set is
47-
// rejected on write by an enforced `Field.select`, so the console would be
48-
// offering something that always 400s.
49-
expect([...ORG_ROLES]).toEqual(['owner', 'admin', 'delegated_admin', 'member']);
50-
});
51-
5242
it('every role has a label — a role with no label renders as a raw key', () => {
5343
for (const role of ORG_ROLES) {
5444
expect(ORG_ROLE_LABELS[role]?.key).toBeTruthy();

packages/auth/src/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ export { createAuthenticatedFetch, ActiveOrganizationStorage, type Authenticated
3838
export { getUserInitials } from './types';
3939

4040
// Organization membership-role vocabulary — a CLOSED, framework-owned list of
41-
// four (framework ADR-0108) — plus the narrowing helpers every org screen
42-
// shares. See `org-roles.ts` for why this still mirrors rather than derives.
41+
// four (framework ADR-0108), re-exported from `@objectstack/spec`'s
42+
// `BUILTIN_MEMBERSHIP_ROLES` so it cannot drift — plus the labels and
43+
// narrowing helpers every org screen shares, which stay console-owned
44+
// (see `org-roles.ts` for the boundary).
4345
export {
4446
ORG_ROLE_OWNER,
4547
ORG_ROLE_ADMIN,

0 commit comments

Comments
 (0)