Skip to content

Commit 069c205

Browse files
os-zhuangclaude
andauthored
fix(spec): view-reference build lint — collision warns, form-target errors (#2554) (#2586)
* chore: bump objectui to 3e4268041bc2 fix(plugin-grid): schema-aware multi-value semantics for bulk-edit params (#2206) objectui@3e4268041bc2523bc3f40a4d54646672ce3394c5 * fix(spec): view-reference build lint — collision warns, form-target errors (#2554) List and form views share one `<object>.<key>` namespace during container expansion, and the default `list` implicitly claims `<object>.default`. A colliding form key was silently renamed to `<key>_2`, so any reference to the requested name (a form action `target`, a nav `viewName`) resolved to the OTHER view — the root cause of a form action opening a list view as a blank form. - spec: `expandViewContainer` gains a behaviour-preserving companion `expandViewContainerWithDiagnostics` reporting every name collision (zero-drift with the runtime expansion). Runtime behaviour unchanged. - cli: new `lint-view-refs` wired into `os compile`, with a broken/fragile severity split tuned NOT to break existing apps on upgrade — form target -> LIST view = ERROR (the concrete #2554 breakage); key collision = WARNING; target -> missing view = WARNING (avoids false positives). Shifts objectui's runtime viewKind guard left to build time. - examples/app-showcase: fix 3 real collisions (formViews.default -> edit), point the Log Time form action at the form view. - tests + api-surface snapshot synced. Verified: cli 466, metadata 257, full turbo build 71/71; revert-proof (bad target -> red, collision-only -> green+warning); 11 downstream apps (hotcrm + templates, 88 view containers) -> 0 false positives, 0 blocked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e695fe0 commit 069c205

12 files changed

Lines changed: 343 additions & 16 deletions

File tree

.changeset/view-ref-build-lint.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/cli": minor
4+
---
5+
6+
Add a build-time view-reference lint that fails `os compile` on a broken form-view reference, and surfaces the previously-silent `_2` rename collision as a warning (#2554).
7+
8+
`expandViewContainer` gains a behaviour-preserving companion `expandViewContainerWithDiagnostics` that also reports every `<object>.<key>` name collision. List and form views share one namespace during expansion, and the default `list` implicitly claims `<object>.default`; a colliding key was previously renamed to `<object>.<key>_2` **silently**, so references (form action `target`s, navigation `viewName`s) resolved to the *other* view.
9+
10+
The new `lint-view-refs` build lint consumes those diagnostics with a broken/fragile severity split, tuned so an upgrade does NOT break existing apps that merely have a colliding key:
11+
12+
- **view-ref-form-target-kind** — ERROR (fails the build): a `type:'form'` action whose `target` resolves to an existing LIST view — the concrete #2554 breakage (a blank form, a silently no-op submit). High-confidence, so it fails.
13+
- **view-key-collision** — WARNING: a key silently renamed on collision. Fragile, not broken — it breaks something only if the requested name is referenced — so it warns.
14+
- **view-ref-form-target-missing** — WARNING: a form target resolving to no view; probably a typo, but possibly a view the lint failed to collect, so it warns rather than risk a false-positive build failure.
15+
16+
This shifts objectui's runtime `viewKind` guard left to compile time: the author — very often an AI generating templates — discovers the mistake on `os compile` instead of when an end user clicks. It mirrors the existing broken/fragile two-level authoring lints (flow-patterns, autonumber, liveness). `expandViewContainer`'s runtime behaviour is unchanged; the fix is diagnostics-only plus the build gate.

examples/app-showcase/src/actions/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,9 @@ export const LogTimeAction = defineAction({
117117
icon: 'clock',
118118
objectName: task,
119119
type: 'form',
120-
// Targets the `edit` form view — `showcase_task.default` is the LIST view
121-
// (the container's main `list` implicitly claims the `default` key).
120+
// Targets the `edit` FORM view. `showcase_task.default` is the LIST view (the
121+
// container's main `list` implicitly claims the `default` key), so pointing a
122+
// form action there opens a list as a form — now a build error (#2554).
122123
target: 'showcase_task.edit',
123124
// `record_section` so it surfaces in the Task Detail quick-actions bar too.
124125
locations: ['record_header', 'record_related', 'record_section'],

examples/app-showcase/src/views/business-unit.view.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ export const BusinessUnitViews = defineView({
3737
},
3838
},
3939
formViews: {
40-
default: {
40+
// `edit`, not `default`: the main `list` implicitly claims `<object>.default`
41+
// in the shared view namespace, so a `default` form key collides (build-time
42+
// view-ref lint, framework #2554).
43+
edit: {
4144
type: 'simple',
4245
data,
4346
sections: [

examples/app-showcase/src/views/project.view.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ export const ProjectViews = defineView({
129129
},
130130
},
131131
formViews: {
132-
default: {
132+
// `edit`, not `default`: the main `list` implicitly claims `<object>.default`
133+
// in the shared view namespace, so a `default` form key collides (build-time
134+
// view-ref lint, framework #2554).
135+
edit: {
133136
type: 'simple',
134137
data,
135138
sections: [

examples/app-showcase/src/views/task.view.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,12 +219,12 @@ export const TaskViews = defineView({
219219
},
220220

221221
formViews: {
222+
// Keyed `edit`, NOT `default`: list and form views share one
223+
// `<object>.<key>` namespace, and the main `list` implicitly claims
224+
// `showcase_task.default`. A `default` form key collides — the build-time
225+
// view-ref lint fails on it (framework #2554) instead of silently renaming
226+
// it to `default_2` and breaking any action target that references it.
222227
// simple ── single-section form ──────────────────────────────────────
223-
// Keyed `edit`, NOT `default`: the container's main `list` implicitly
224-
// claims `showcase_task.default`, and list + form views share one
225-
// `<object>.<key>` namespace — a `default` form key would be renamed to
226-
// `default_2` at expansion (with a boot warning), breaking any action
227-
// `target` that references it. See framework issue #2554.
228228
edit: {
229229
type: 'simple',
230230
data,

packages/cli/src/commands/compile.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { validateResponsiveStyles } from '@objectstack/lint';
1414
import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
1515
import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
1616
import { lintLivenessProperties } from '../utils/lint-liveness-properties.js';
17+
import { lintViewRefs } from '../utils/lint-view-refs.js';
1718
import { collectAndLintDocs } from '../utils/collect-docs.js';
1819
import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js';
1920
import {
@@ -290,6 +291,38 @@ export default class Compile extends Command {
290291
}
291292
}
292293

294+
// 3d-quater. View-reference lint (#2554) — resolves form action targets
295+
// and view-key collisions at build time. A `type:'form'` target that
296+
// names a missing view or a LIST view opens a broken/blank form at
297+
// runtime; a list/form key collision silently renames one view so
298+
// references resolve to the OTHER. Both are broken → fail the build.
299+
// This shifts objectui's runtime `viewKind` guard left to compile.
300+
const viewRefLint = lintViewRefs(result.data as Record<string, unknown>);
301+
const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error');
302+
const viewRefWarnings = viewRefLint.filter((f) => f.severity === 'warning');
303+
if (viewRefErrors.length > 0) {
304+
if (flags.json) {
305+
console.log(JSON.stringify({ success: false, error: 'view reference validation failed', issues: viewRefErrors }));
306+
this.exit(1);
307+
}
308+
console.log('');
309+
printError(`View reference validation failed (${viewRefErrors.length} issue${viewRefErrors.length > 1 ? 's' : ''})`);
310+
for (const f of viewRefErrors) {
311+
console.log(` • ${f.where}: ${f.message}`);
312+
console.log(chalk.dim(` ${f.hint}`));
313+
console.log(chalk.dim(` rule: ${f.rule}`));
314+
}
315+
this.exit(1);
316+
}
317+
if (viewRefWarnings.length > 0 && !flags.json) {
318+
console.log('');
319+
for (const f of viewRefWarnings) {
320+
printWarning(`${f.where}: ${f.message}`);
321+
console.log(chalk.dim(` ${f.hint}`));
322+
console.log(chalk.dim(` rule: ${f.rule}`));
323+
}
324+
}
325+
293326
// 3d. Package docs (ADR-0046): compile flat `src/docs/*.md` into
294327
// `docs: DocSchema[]` and lint the combined set (flatness,
295328
// namespace-prefixed names, MDX/image ban, same-package link
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
lintViewRefs,
6+
VIEW_KEY_COLLISION,
7+
VIEW_REF_FORM_TARGET_MISSING,
8+
VIEW_REF_FORM_TARGET_KIND,
9+
} from './lint-view-refs.js';
10+
11+
const listView = (object: string) => ({
12+
type: 'grid',
13+
label: 'All',
14+
columns: ['title'],
15+
data: { provider: 'object', object },
16+
});
17+
const formView = (object: string) => ({
18+
type: 'simple',
19+
data: { provider: 'object', object },
20+
sections: [],
21+
});
22+
23+
describe('lintViewRefs — clean paths', () => {
24+
it('passes a container whose form key does not collide, with a correct form target', () => {
25+
const stack = {
26+
views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }],
27+
actions: [{ name: 'log_time', type: 'form', target: 'task.edit' }],
28+
};
29+
expect(lintViewRefs(stack)).toEqual([]);
30+
});
31+
32+
it('ignores non-form action types (their target is not a form-view ref)', () => {
33+
const stack = {
34+
views: [{ name: 'task', list: listView('task') }],
35+
actions: [
36+
{ name: 'open_docs', type: 'url', target: 'https://example.com' },
37+
{ name: 'gallery', type: 'modal', target: 'some_modal' },
38+
],
39+
};
40+
expect(lintViewRefs(stack)).toEqual([]);
41+
});
42+
43+
it('skips dynamic (interpolated) and non-qualified targets', () => {
44+
const stack = {
45+
views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }],
46+
actions: [
47+
{ name: 'a', type: 'form', target: 'task.${param.view}' },
48+
{ name: 'b', type: 'form', target: 'bare_key_no_dot' },
49+
],
50+
};
51+
expect(lintViewRefs(stack)).toEqual([]);
52+
});
53+
});
54+
55+
describe('lintViewRefs — object name derivation (real defineView shape)', () => {
56+
// `defineView({...})` containers carry NO top-level name/object — the object
57+
// lives only in `list.data.object`. The lint must derive it exactly like the
58+
// runtime loader, or every name-less container silently drops out of the index
59+
// and its targets read as "missing" (the false negative found dogfooding
60+
// app-showcase).
61+
it('indexes a name-less container via list.data.object and accepts a good target', () => {
62+
const stack = {
63+
views: [{ list: listView('showcase_task'), formViews: { edit: formView('showcase_task') } }],
64+
actions: [{ name: 'log_time', type: 'form', target: 'showcase_task.edit' }],
65+
};
66+
expect(lintViewRefs(stack)).toEqual([]);
67+
});
68+
69+
it('still flags a genuinely missing target on a name-less container', () => {
70+
const stack = {
71+
views: [{ list: listView('showcase_task'), formViews: { edit: formView('showcase_task') } }],
72+
actions: [{ name: 'x', type: 'form', target: 'showcase_task.nope' }],
73+
};
74+
expect(lintViewRefs(stack).some((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING)).toBe(true);
75+
});
76+
});
77+
78+
describe('lintViewRefs — view-key collisions (#2554)', () => {
79+
it('warns (does NOT fail the build) when formViews.default collides with the implicit default list', () => {
80+
const stack = {
81+
views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }],
82+
};
83+
const out = lintViewRefs(stack);
84+
const collision = out.find((f) => f.rule === VIEW_KEY_COLLISION);
85+
expect(collision).toBeDefined();
86+
// Fragile, not broken: a rename only breaks something if the name is referenced.
87+
expect(collision!.severity).toBe('warning');
88+
expect(collision!.message).toContain("'task.default'");
89+
expect(collision!.message).toContain("'task.default_2'");
90+
});
91+
92+
it('detects the collision in object-nested listViews/formViews too', () => {
93+
const stack = {
94+
objects: [
95+
{
96+
name: 'task',
97+
listViews: { mine: listView('task') },
98+
formViews: { mine: formView('task') },
99+
},
100+
],
101+
};
102+
const out = lintViewRefs(stack);
103+
expect(out.some((f) => f.rule === VIEW_KEY_COLLISION && f.message.includes("'task.mine'"))).toBe(true);
104+
});
105+
106+
it('a collision-only stack yields NO error-severity finding — build is not blocked', () => {
107+
const stack = {
108+
views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }],
109+
};
110+
const out = lintViewRefs(stack);
111+
expect(out.some((f) => f.rule === VIEW_KEY_COLLISION)).toBe(true); // collision surfaced…
112+
expect(out.some((f) => f.severity === 'error')).toBe(false); // …but nothing fails the build
113+
});
114+
});
115+
116+
describe('lintViewRefs — form action target resolution', () => {
117+
it('errors when a form target names a LIST view (the #2554 runtime symptom)', () => {
118+
// `default` is the list; the form collides to `default_2`, so `task.default`
119+
// resolves to the list — exactly what opened a blank form at runtime.
120+
const stack = {
121+
views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }],
122+
actions: [{ name: 'log_time', type: 'form', target: 'task.default' }],
123+
};
124+
const out = lintViewRefs(stack);
125+
const kindErr = out.find((f) => f.rule === VIEW_REF_FORM_TARGET_KIND);
126+
expect(kindErr).toBeDefined();
127+
expect(kindErr!.severity).toBe('error');
128+
expect(kindErr!.message).toContain('list view, not a form view');
129+
});
130+
131+
it('warns (possible false positive, does NOT fail) when a form target resolves to no view at all', () => {
132+
const stack = {
133+
views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }],
134+
actions: [{ name: 'log_time', type: 'form', target: 'task.nope' }],
135+
};
136+
const out = lintViewRefs(stack);
137+
const missWarn = out.find((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING);
138+
expect(missWarn).toBeDefined();
139+
// Might be a view the lint failed to collect — warn rather than break the build.
140+
expect(missWarn!.severity).toBe('warning');
141+
});
142+
143+
it('accepts a form target that resolves to an actual form view', () => {
144+
const stack = {
145+
views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }],
146+
actions: [{ name: 'log_time', type: 'form', target: 'task.edit' }],
147+
};
148+
expect(lintViewRefs(stack).filter((f) => f.rule.startsWith('view-ref'))).toEqual([]);
149+
});
150+
151+
it('validates object-nested actions against object-nested form views', () => {
152+
const stack = {
153+
objects: [
154+
{
155+
name: 'task',
156+
formViews: { edit: formView('task') },
157+
actions: [{ name: 'nested_form', type: 'form', target: 'task.missing' }],
158+
},
159+
],
160+
};
161+
const out = lintViewRefs(stack);
162+
expect(out.some((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING && f.where.includes("object 'task'"))).toBe(true);
163+
});
164+
165+
it('reports a shared (top-level + object-nested) action only once', () => {
166+
const shared = { name: 'log_time', type: 'form', target: 'task.missing' };
167+
const stack = {
168+
views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }],
169+
actions: [shared],
170+
objects: [{ name: 'task', actions: [shared] }],
171+
};
172+
const out = lintViewRefs(stack).filter((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING);
173+
expect(out).toHaveLength(1);
174+
expect(out[0].where).toContain("object 'task'"); // object-nested context retained
175+
});
176+
});
8.92 KB
Binary file not shown.

packages/metadata/src/view-expand.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { describe, it, expect } from 'vitest';
44
import { expandViewContainer, isAggregatedViewContainer } from './plugin.js';
5+
import { expandViewContainerWithDiagnostics } from '@objectstack/spec';
56

67
// Mirrors examples/app-crm/src/views/lead.view.ts — the canonical case where
78
// the default `list` and `listViews.all` are structurally identical (the
@@ -104,6 +105,46 @@ describe('expandViewContainer — default list with no listViews dup', () => {
104105
});
105106
});
106107

108+
describe('expandViewContainerWithDiagnostics — collision capture (#2554)', () => {
109+
it('is behaviour-preserving: items match expandViewContainer, no false collisions', () => {
110+
const { items, collisions } = expandViewContainerWithDiagnostics('crm_lead', leadContainer);
111+
// The deduped lead container has no real collision (list folds into `all`,
112+
// so `default` is free for the form).
113+
expect(collisions).toEqual([]);
114+
expect(items.map((i) => i.name)).toEqual(
115+
expandViewContainer('crm_lead', leadContainer).map((i) => i.name),
116+
);
117+
});
118+
119+
it('captures a formViews key colliding with the implicit default list', () => {
120+
const { items, collisions } = expandViewContainerWithDiagnostics('task', {
121+
list: { type: 'grid', label: 'All', columns: ['title'], data: { provider: 'object', object: 'task' } },
122+
formViews: { default: { type: 'simple', data: { provider: 'object', object: 'task' }, sections: [] } },
123+
});
124+
// Rename behaviour is unchanged (backward compat): list keeps the name,
125+
// the form loses and is renamed.
126+
expect(items.find((i) => i.viewKind === 'list')!.name).toBe('task.default');
127+
expect(items.find((i) => i.viewKind === 'form')!.name).toBe('task.default_2');
128+
// …and the collision is captured, naming both sides + the losing kind.
129+
expect(collisions).toHaveLength(1);
130+
expect(collisions[0]).toMatchObject({
131+
requested: 'task.default',
132+
renamedTo: 'task.default_2',
133+
viewKind: 'form',
134+
key: 'default',
135+
});
136+
});
137+
138+
it('captures a formViews key colliding with a listViews key', () => {
139+
const { collisions } = expandViewContainerWithDiagnostics('task', {
140+
listViews: { mine: { type: 'grid', label: 'Mine', columns: ['title'], data: { provider: 'object', object: 'task' } } },
141+
formViews: { mine: { type: 'simple', data: { provider: 'object', object: 'task' }, sections: [] } },
142+
});
143+
expect(collisions.map((c) => c.requested)).toContain('task.mine');
144+
expect(collisions.find((c) => c.requested === 'task.mine')!.viewKind).toBe('form');
145+
});
146+
});
147+
107148
describe('expandViewContainer — name collisions carry _diagnostics warnings (#2554)', () => {
108149
it('warns when formViews.default collides with the implicit default list', () => {
109150
const items = expandViewContainer('task', {

packages/spec/api-surface.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"EvalUser (type)",
2121
"EvalUserInput (type)",
2222
"EvalUserSchema (const)",
23+
"ExpandViewResult (interface)",
2324
"ExpandedViewItem (interface)",
2425
"Expression (type)",
2526
"ExpressionDialect (type)",
@@ -55,6 +56,7 @@
5556
"Skill (type)",
5657
"TemplateExpressionInputSchema (const)",
5758
"Tool (type)",
59+
"ViewKeyCollision (interface)",
5860
"cel (function)",
5961
"composeStacks (function)",
6062
"createEvalUser (function)",
@@ -86,6 +88,7 @@
8688
"defineViewItem (function)",
8789
"defineWebhook (function)",
8890
"expandViewContainer (function)",
91+
"expandViewContainerWithDiagnostics (function)",
8992
"expression (function)",
9093
"findClosestMatches (function)",
9194
"formatSuggestion (function)",
@@ -3149,6 +3152,7 @@
31493152
"EmbedConfigSchema (const)",
31503153
"EvictionPolicy (type)",
31513154
"EvictionPolicySchema (const)",
3155+
"ExpandViewResult (interface)",
31523156
"ExpandedViewItem (interface)",
31533157
"FieldWidgetProps (type)",
31543158
"FieldWidgetPropsSchema (const)",
@@ -3383,6 +3387,7 @@
33833387
"ViewItem (type)",
33843388
"ViewItemNameSchema (const)",
33853389
"ViewItemSchema (const)",
3390+
"ViewKeyCollision (interface)",
33863391
"ViewKind (type)",
33873392
"ViewKindSchema (const)",
33883393
"ViewSchema (const)",
@@ -3427,6 +3432,7 @@
34273432
"defineView (function)",
34283433
"defineViewItem (function)",
34293434
"expandViewContainer (function)",
3435+
"expandViewContainerWithDiagnostics (function)",
34303436
"isAggregatedViewContainer (function)",
34313437
"pageForm (const)",
34323438
"reportForm (const)",

0 commit comments

Comments
 (0)