From 8e661d544174e02b7120f016d6930a1dc15c74dd Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sat, 4 Jul 2026 11:05:32 +0800 Subject: [PATCH 1/2] chore: bump objectui to 3e4268041bc2 fix(plugin-grid): schema-aware multi-value semantics for bulk-edit params (#2206) objectui@3e4268041bc2523bc3f40a4d54646672ce3394c5 --- .objectui-sha | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.objectui-sha b/.objectui-sha index 66c3debb65..c275227bec 100644 --- a/.objectui-sha +++ b/.objectui-sha @@ -1 +1 @@ -09e1b261563284fbeddc7c5c1bde753330ccab87 \ No newline at end of file +3e4268041bc2523bc3f40a4d54646672ce3394c5 From 6ffb7ed2d9ca576725a079c785d3d47c579142f1 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sat, 4 Jul 2026 23:12:32 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix(spec):=20view-reference=20build=20lint?= =?UTF-8?q?=20=E2=80=94=20collision=20warns,=20form-target=20errors=20(#25?= =?UTF-8?q?54)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List and form views share one `.` namespace during container expansion, and the default `list` implicitly claims `.default`. A colliding form key was silently renamed to `_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 --- .changeset/view-ref-build-lint.md | 16 ++ examples/app-showcase/src/actions/index.ts | 5 +- .../src/views/business-unit.view.ts | 5 +- .../app-showcase/src/views/project.view.ts | 5 +- examples/app-showcase/src/views/task.view.ts | 7 +- packages/cli/src/commands/compile.ts | 33 ++++ packages/cli/src/utils/lint-view-refs.test.ts | 176 ++++++++++++++++++ packages/cli/src/utils/lint-view-refs.ts | Bin 0 -> 9137 bytes packages/metadata/src/view-expand.test.ts | 41 ++++ packages/spec/api-surface.json | 6 + packages/spec/src/index.ts | 4 +- packages/spec/src/ui/view.zod.ts | 70 ++++++- 12 files changed, 353 insertions(+), 15 deletions(-) create mode 100644 .changeset/view-ref-build-lint.md create mode 100644 packages/cli/src/utils/lint-view-refs.test.ts create mode 100644 packages/cli/src/utils/lint-view-refs.ts diff --git a/.changeset/view-ref-build-lint.md b/.changeset/view-ref-build-lint.md new file mode 100644 index 0000000000..dccd387972 --- /dev/null +++ b/.changeset/view-ref-build-lint.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": minor +"@objectstack/cli": minor +--- + +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). + +`expandViewContainer` gains a behaviour-preserving companion `expandViewContainerWithDiagnostics` that also reports every `.` name collision. List and form views share one namespace during expansion, and the default `list` implicitly claims `.default`; a colliding key was previously renamed to `._2` **silently**, so references (form action `target`s, navigation `viewName`s) resolved to the *other* view. + +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: + +- **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. +- **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. +- **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. + +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. diff --git a/examples/app-showcase/src/actions/index.ts b/examples/app-showcase/src/actions/index.ts index b263d1ff1b..f9cf1ddd98 100644 --- a/examples/app-showcase/src/actions/index.ts +++ b/examples/app-showcase/src/actions/index.ts @@ -117,7 +117,10 @@ export const LogTimeAction = defineAction({ icon: 'clock', objectName: task, type: 'form', - target: 'showcase_task.default', + // Targets the `edit` FORM view. `showcase_task.default` is the LIST view (the + // container's main `list` implicitly claims the `default` key), so pointing a + // form action there opens a list as a form — now a build error (#2554). + target: 'showcase_task.edit', // `record_section` so it surfaces in the Task Detail quick-actions bar too. locations: ['record_header', 'record_related', 'record_section'], refreshAfter: true, diff --git a/examples/app-showcase/src/views/business-unit.view.ts b/examples/app-showcase/src/views/business-unit.view.ts index 9ee3f190ac..7762cbbe3d 100644 --- a/examples/app-showcase/src/views/business-unit.view.ts +++ b/examples/app-showcase/src/views/business-unit.view.ts @@ -37,7 +37,10 @@ export const BusinessUnitViews = defineView({ }, }, formViews: { - default: { + // `edit`, not `default`: the main `list` implicitly claims `.default` + // in the shared view namespace, so a `default` form key collides (build-time + // view-ref lint, framework #2554). + edit: { type: 'simple', data, sections: [ diff --git a/examples/app-showcase/src/views/project.view.ts b/examples/app-showcase/src/views/project.view.ts index 51aa1933bc..7baf77729a 100644 --- a/examples/app-showcase/src/views/project.view.ts +++ b/examples/app-showcase/src/views/project.view.ts @@ -129,7 +129,10 @@ export const ProjectViews = defineView({ }, }, formViews: { - default: { + // `edit`, not `default`: the main `list` implicitly claims `.default` + // in the shared view namespace, so a `default` form key collides (build-time + // view-ref lint, framework #2554). + edit: { type: 'simple', data, sections: [ diff --git a/examples/app-showcase/src/views/task.view.ts b/examples/app-showcase/src/views/task.view.ts index 0d2c243a0c..f775a93de3 100644 --- a/examples/app-showcase/src/views/task.view.ts +++ b/examples/app-showcase/src/views/task.view.ts @@ -229,8 +229,13 @@ export const TaskViews = defineView({ }, formViews: { + // Keyed `edit`, NOT `default`: list and form views share one + // `.` namespace, and the main `list` implicitly claims + // `showcase_task.default`. A `default` form key collides — the build-time + // view-ref lint fails on it (framework #2554) instead of silently renaming + // it to `default_2` and breaking any action target that references it. // simple ── single-section form ────────────────────────────────────── - default: { + edit: { type: 'simple', data, sections: [ diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 7df7d70aad..00fda324c8 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -14,6 +14,7 @@ import { validateResponsiveStyles } from '@objectstack/lint'; import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; +import { lintViewRefs } from '../utils/lint-view-refs.js'; import { collectAndLintDocs } from '../utils/collect-docs.js'; import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js'; import { @@ -290,6 +291,38 @@ export default class Compile extends Command { } } + // 3d-quater. View-reference lint (#2554) — resolves form action targets + // and view-key collisions at build time. A `type:'form'` target that + // names a missing view or a LIST view opens a broken/blank form at + // runtime; a list/form key collision silently renames one view so + // references resolve to the OTHER. Both are broken → fail the build. + // This shifts objectui's runtime `viewKind` guard left to compile. + const viewRefLint = lintViewRefs(result.data as Record); + const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error'); + const viewRefWarnings = viewRefLint.filter((f) => f.severity === 'warning'); + if (viewRefErrors.length > 0) { + if (flags.json) { + console.log(JSON.stringify({ success: false, error: 'view reference validation failed', issues: viewRefErrors })); + this.exit(1); + } + console.log(''); + printError(`View reference validation failed (${viewRefErrors.length} issue${viewRefErrors.length > 1 ? 's' : ''})`); + for (const f of viewRefErrors) { + console.log(` • ${f.where}: ${f.message}`); + console.log(chalk.dim(` ${f.hint}`)); + console.log(chalk.dim(` rule: ${f.rule}`)); + } + this.exit(1); + } + if (viewRefWarnings.length > 0 && !flags.json) { + console.log(''); + for (const f of viewRefWarnings) { + printWarning(`${f.where}: ${f.message}`); + console.log(chalk.dim(` ${f.hint}`)); + console.log(chalk.dim(` rule: ${f.rule}`)); + } + } + // 3d. Package docs (ADR-0046): compile flat `src/docs/*.md` into // `docs: DocSchema[]` and lint the combined set (flatness, // namespace-prefixed names, MDX/image ban, same-package link diff --git a/packages/cli/src/utils/lint-view-refs.test.ts b/packages/cli/src/utils/lint-view-refs.test.ts new file mode 100644 index 0000000000..f7ec14e8f9 --- /dev/null +++ b/packages/cli/src/utils/lint-view-refs.test.ts @@ -0,0 +1,176 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + lintViewRefs, + VIEW_KEY_COLLISION, + VIEW_REF_FORM_TARGET_MISSING, + VIEW_REF_FORM_TARGET_KIND, +} from './lint-view-refs.js'; + +const listView = (object: string) => ({ + type: 'grid', + label: 'All', + columns: ['title'], + data: { provider: 'object', object }, +}); +const formView = (object: string) => ({ + type: 'simple', + data: { provider: 'object', object }, + sections: [], +}); + +describe('lintViewRefs — clean paths', () => { + it('passes a container whose form key does not collide, with a correct form target', () => { + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }], + actions: [{ name: 'log_time', type: 'form', target: 'task.edit' }], + }; + expect(lintViewRefs(stack)).toEqual([]); + }); + + it('ignores non-form action types (their target is not a form-view ref)', () => { + const stack = { + views: [{ name: 'task', list: listView('task') }], + actions: [ + { name: 'open_docs', type: 'url', target: 'https://example.com' }, + { name: 'gallery', type: 'modal', target: 'some_modal' }, + ], + }; + expect(lintViewRefs(stack)).toEqual([]); + }); + + it('skips dynamic (interpolated) and non-qualified targets', () => { + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }], + actions: [ + { name: 'a', type: 'form', target: 'task.${param.view}' }, + { name: 'b', type: 'form', target: 'bare_key_no_dot' }, + ], + }; + expect(lintViewRefs(stack)).toEqual([]); + }); +}); + +describe('lintViewRefs — object name derivation (real defineView shape)', () => { + // `defineView({...})` containers carry NO top-level name/object — the object + // lives only in `list.data.object`. The lint must derive it exactly like the + // runtime loader, or every name-less container silently drops out of the index + // and its targets read as "missing" (the false negative found dogfooding + // app-showcase). + it('indexes a name-less container via list.data.object and accepts a good target', () => { + const stack = { + views: [{ list: listView('showcase_task'), formViews: { edit: formView('showcase_task') } }], + actions: [{ name: 'log_time', type: 'form', target: 'showcase_task.edit' }], + }; + expect(lintViewRefs(stack)).toEqual([]); + }); + + it('still flags a genuinely missing target on a name-less container', () => { + const stack = { + views: [{ list: listView('showcase_task'), formViews: { edit: formView('showcase_task') } }], + actions: [{ name: 'x', type: 'form', target: 'showcase_task.nope' }], + }; + expect(lintViewRefs(stack).some((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING)).toBe(true); + }); +}); + +describe('lintViewRefs — view-key collisions (#2554)', () => { + it('warns (does NOT fail the build) when formViews.default collides with the implicit default list', () => { + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }], + }; + const out = lintViewRefs(stack); + const collision = out.find((f) => f.rule === VIEW_KEY_COLLISION); + expect(collision).toBeDefined(); + // Fragile, not broken: a rename only breaks something if the name is referenced. + expect(collision!.severity).toBe('warning'); + expect(collision!.message).toContain("'task.default'"); + expect(collision!.message).toContain("'task.default_2'"); + }); + + it('detects the collision in object-nested listViews/formViews too', () => { + const stack = { + objects: [ + { + name: 'task', + listViews: { mine: listView('task') }, + formViews: { mine: formView('task') }, + }, + ], + }; + const out = lintViewRefs(stack); + expect(out.some((f) => f.rule === VIEW_KEY_COLLISION && f.message.includes("'task.mine'"))).toBe(true); + }); + + it('a collision-only stack yields NO error-severity finding — build is not blocked', () => { + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }], + }; + const out = lintViewRefs(stack); + expect(out.some((f) => f.rule === VIEW_KEY_COLLISION)).toBe(true); // collision surfaced… + expect(out.some((f) => f.severity === 'error')).toBe(false); // …but nothing fails the build + }); +}); + +describe('lintViewRefs — form action target resolution', () => { + it('errors when a form target names a LIST view (the #2554 runtime symptom)', () => { + // `default` is the list; the form collides to `default_2`, so `task.default` + // resolves to the list — exactly what opened a blank form at runtime. + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { default: formView('task') } }], + actions: [{ name: 'log_time', type: 'form', target: 'task.default' }], + }; + const out = lintViewRefs(stack); + const kindErr = out.find((f) => f.rule === VIEW_REF_FORM_TARGET_KIND); + expect(kindErr).toBeDefined(); + expect(kindErr!.severity).toBe('error'); + expect(kindErr!.message).toContain('list view, not a form view'); + }); + + it('warns (possible false positive, does NOT fail) when a form target resolves to no view at all', () => { + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }], + actions: [{ name: 'log_time', type: 'form', target: 'task.nope' }], + }; + const out = lintViewRefs(stack); + const missWarn = out.find((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING); + expect(missWarn).toBeDefined(); + // Might be a view the lint failed to collect — warn rather than break the build. + expect(missWarn!.severity).toBe('warning'); + }); + + it('accepts a form target that resolves to an actual form view', () => { + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }], + actions: [{ name: 'log_time', type: 'form', target: 'task.edit' }], + }; + expect(lintViewRefs(stack).filter((f) => f.rule.startsWith('view-ref'))).toEqual([]); + }); + + it('validates object-nested actions against object-nested form views', () => { + const stack = { + objects: [ + { + name: 'task', + formViews: { edit: formView('task') }, + actions: [{ name: 'nested_form', type: 'form', target: 'task.missing' }], + }, + ], + }; + const out = lintViewRefs(stack); + expect(out.some((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING && f.where.includes("object 'task'"))).toBe(true); + }); + + it('reports a shared (top-level + object-nested) action only once', () => { + const shared = { name: 'log_time', type: 'form', target: 'task.missing' }; + const stack = { + views: [{ name: 'task', list: listView('task'), formViews: { edit: formView('task') } }], + actions: [shared], + objects: [{ name: 'task', actions: [shared] }], + }; + const out = lintViewRefs(stack).filter((f) => f.rule === VIEW_REF_FORM_TARGET_MISSING); + expect(out).toHaveLength(1); + expect(out[0].where).toContain("object 'task'"); // object-nested context retained + }); +}); diff --git a/packages/cli/src/utils/lint-view-refs.ts b/packages/cli/src/utils/lint-view-refs.ts new file mode 100644 index 0000000000000000000000000000000000000000..9d48795b1f03e3b5de1b6cb2f4ac3d8d6c6918fb GIT binary patch literal 9137 zcmcgy-E!N;74Eg3Vx49}(j_6sZKgA_C0DU!S0h`>$Qoxl8IQRnmLx(DV6XrsRnbhY z`T(6iLtdfxeUv;&zwev{2vTz6UbMRKNWkvdv*�J3sL5u6dp>uZv_lD@}LYGmjrV z{>q$)N&tP*!=Y!;h5ORvquCMn++RVL?ltr?{@o9~W_eC{%l**DO` zQk+jx2YFdO$d|@fqeW8oB6BfIyz#SSQhJli^{b@g8<9hvc#>rC&`c{^#3pqUD3ntm zEi&S*`{5RRV5@S56M55>E3QpGDWQqY%;B+_y37@}OmLCXEtaV*VF;|GusHGK96Qu9 zi^P|9?r=>r%)J@sizT!lnk4h3v$+0*M~YkrYg8T%8p9=XA3+%9y>$?llrU1Brun6c zRlDp?3Oj{3%}&oQ%&2g7PI>MMD)E(FE-4IVU%+$l z!pyGdz%kBAWb@oz%l*Yi*35vh!8p$*N$j$*>r)ymQ6(964}z--H-R*~7N-VnY1_SU zA0xgWVBLUF8I-n|x^ggwYviz_^YgQF(*%;~b`&aaQ(UdZ{ixO|Dwz@-Ilirp?@X#=nb;Q%;TNFdj1AvM4*@wGf6 z2PPU9u5{+NkH7lr4{CWkg=rxd7~M7_9hM->HZNSeKbQ-yfa&2F&$#M}pl4KAgl3+>KuPHnRAE#YD5FoL@pIMo^wRWJiUg0o}Oy`$4FfBSn& zmMHq+@vuibQIEn+p#UNhf@+xvvTF~1OMa*veZo}~Z*0nI<2b?a!w@hJ#d0vZL&_&E-m;7m7I<~}JCkc^l`3{@4b zHqQ%}CL_`+xd;b=3RmD*EQdmK=awo3*+ivylQ~v&2nUdvF{^34yRMZ70=#S zX>QG=$QP#bC*=(QVLadUOE>O3+1UYC@C>ocgdD=X=Wg;6Tuwya>=;d`dj3HeS?4VB7Ul8 z()`y4&=%#5IfYN*1EdNIlX2A`F9r*gFCYLL+Y-E==5i%T9JVOzHSCkIoZ^*+M8{B! zA|XI9aXU$^yGW>=X04e^mtce>6CUu%NH{@@3JPLrVJmWxn`AppHsoW%l>?O-EhX zY4)8-wG^SIDiY_hqY=}KaJO#vJ6^ia1n|CR_76<=MuMboqA2R(X={VI?e%zcQ0cq( z;+_B$26ZX5k!BTSaF@@;Mu$TP=kJWd3OHh!^PqGtD}n# zZ;s!-MfPhXe1u#-N!H2n>5GpFVtoQBf}(G#G!?gs(hVmHbP%{8To@afKn+d1B^ij1 ziTm)y`QXu`FaHScLIEg5U_Mg^Ak;mem|aU4eFkhCjywxpA`&?YDNZVQ2m?bfTM&Gb zuUXb72xsJk8kLJpXf;Z{L^$htU8q;XtHGI|EI985AoU1Ltc&(pCNx$s zag!R(#1nGS1UGKsL)jF}%PvweNPZ{BkbQx8Zir}fr5WAOA^$XQ8qR5+)I8Oek;L~) zNaD@?Nqpb#04AZjZ5{O8rWAd4pCtP1Swv?&i|&!SDTa$>ZCHArKzjd^1#L@VO^`W?xI!)M)?Q(%ET^cY# zMx==AVMP#&8rP$m^en$1YGOht;<;ULBEit%1z12y`B};Z zPlRyx!Pl4Oja@cH1upZnL!V1ALSJFB51IzglI%JU{@-^rlz|iXjJ}l^)E_YH}XxQy(Ha5mbHjcZzpNIeptF^xD znOEAo(30p4nmTYzx_fO><&l19eit}yDrl0zH!o)Zh`2r>aIOursg?o;E-t4G-4DLzI}bZ;zmZ4`SC{) zLc7<$^Z7dhw|>Bv@?eb_8uVA~&@9?4qk6$SY9RC@0>Q_)?>1Mo`2Lo~L`evgaLs38M4>VX?HLe%&-l7Z3aHPSsc?0-UUNV6g!T3`qe zYN>;;3E3aV%GDT+U64L+i0-LuLgMabXr!8l*;b=DZVV~RM*)|xf&R#(c@Ohctr|O@ z-{^0*9rLe$3ihQP!*d9aP#xWNhG^l#Pu+hu4~!WS3bLoM4>%9`KqZiKc%xgD9pn}c z?Kx|B9E$;%pTU<>xLKq)4uiPOfGOJt)$JgQq{dm2&o1HadL)J_vN?n%j4$%r4l-zV ztwXB1*_VcN^MGdGS~<*cBo@Lf|j3s33WZiNzC2r$tg{6cJb$cAp-M*)rRUVOEWQU_qRToH5_nb@ z7p*Ev@Y2 z)7mCtq9hxqRg5_cAlhk$H&5uj_?ieFgG1|4BV&^hLQA3m!n7`Qatu<8d7VU^+*FTk ziI29VZ%iOXYHQlaX@w3P5en)#9S<{?)%CENkUW9h5v&l*N^ER4ZO>}+$t#pS-74d+ zaSvmZwM@m=2|{+?V3@i-;`TS6-{_rghifBl;U_}Zj|mhZ3K45-$^qMJSpl!tww2It zm-}r*p%fXaGUSq$S{;Bplt_Hj0wK~2%Da#3!W~+-W+8sJ!Vkz<-}Zs5t8@9UWW<_j z)+1Iqkfuh8W6%YUgy}W44+ie&cS52p@INOR97T~|*t^l&;hS46Wl@2Qnoql1WX#H? z{?I7dhk~B8zMO?b(*FF`{@UB-T^$b(pL2jOWl;TTgCB%7w+Ri_=@VRKH?w)fLJVaL z$*Q$M;AqyV)%@ab0d=6KweOXi07MLP z*eqf8Dy!c|-{bRox3k*`#nt~3Chh9LqC8S{v@oh!qj7g+&fs2V)M zS?!zA@vkM zlW6GWQ7#pT>@bx<7Gqb0?`BG!=(0myju3PB9|WNaXEp1tqz-}Vbq9}^|Gx&Yb#6cq u$M9i%08Ju&%FeV^S%k6z<%Coc_qDlqrRgx@Kgoj}&~KR6)twB-b^iuijn^#z literal 0 HcmV?d00001 diff --git a/packages/metadata/src/view-expand.test.ts b/packages/metadata/src/view-expand.test.ts index ef128c6eb7..ac71c28a16 100644 --- a/packages/metadata/src/view-expand.test.ts +++ b/packages/metadata/src/view-expand.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { expandViewContainer, isAggregatedViewContainer } from './plugin.js'; +import { expandViewContainerWithDiagnostics } from '@objectstack/spec'; // Mirrors examples/app-crm/src/views/lead.view.ts — the canonical case where // the default `list` and `listViews.all` are structurally identical (the @@ -103,3 +104,43 @@ describe('expandViewContainer — default list with no listViews dup', () => { expect(items[0].isDefault).toBe(true); }); }); + +describe('expandViewContainerWithDiagnostics — collision capture (#2554)', () => { + it('is behaviour-preserving: items match expandViewContainer, no false collisions', () => { + const { items, collisions } = expandViewContainerWithDiagnostics('crm_lead', leadContainer); + // The deduped lead container has no real collision (list folds into `all`, + // so `default` is free for the form). + expect(collisions).toEqual([]); + expect(items.map((i) => i.name)).toEqual( + expandViewContainer('crm_lead', leadContainer).map((i) => i.name), + ); + }); + + it('captures a formViews key colliding with the implicit default list', () => { + const { items, collisions } = expandViewContainerWithDiagnostics('task', { + list: { type: 'grid', label: 'All', columns: ['title'], data: { provider: 'object', object: 'task' } }, + formViews: { default: { type: 'simple', data: { provider: 'object', object: 'task' }, sections: [] } }, + }); + // Rename behaviour is unchanged (backward compat): list keeps the name, + // the form loses and is renamed. + expect(items.find((i) => i.viewKind === 'list')!.name).toBe('task.default'); + expect(items.find((i) => i.viewKind === 'form')!.name).toBe('task.default_2'); + // …and the collision is captured, naming both sides + the losing kind. + expect(collisions).toHaveLength(1); + expect(collisions[0]).toMatchObject({ + requested: 'task.default', + renamedTo: 'task.default_2', + viewKind: 'form', + key: 'default', + }); + }); + + it('captures a formViews key colliding with a listViews key', () => { + const { collisions } = expandViewContainerWithDiagnostics('task', { + listViews: { mine: { type: 'grid', label: 'Mine', columns: ['title'], data: { provider: 'object', object: 'task' } } }, + formViews: { mine: { type: 'simple', data: { provider: 'object', object: 'task' }, sections: [] } }, + }); + expect(collisions.map((c) => c.requested)).toContain('task.mine'); + expect(collisions.find((c) => c.requested === 'task.mine')!.viewKind).toBe('form'); + }); +}); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 3bb3f4019e..397f00cbf2 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -20,6 +20,7 @@ "EvalUser (type)", "EvalUserInput (type)", "EvalUserSchema (const)", + "ExpandViewResult (interface)", "ExpandedViewItem (interface)", "Expression (type)", "ExpressionDialect (type)", @@ -55,6 +56,7 @@ "Skill (type)", "TemplateExpressionInputSchema (const)", "Tool (type)", + "ViewKeyCollision (interface)", "cel (function)", "composeStacks (function)", "createEvalUser (function)", @@ -86,6 +88,7 @@ "defineViewItem (function)", "defineWebhook (function)", "expandViewContainer (function)", + "expandViewContainerWithDiagnostics (function)", "expression (function)", "findClosestMatches (function)", "formatSuggestion (function)", @@ -3149,6 +3152,7 @@ "EmbedConfigSchema (const)", "EvictionPolicy (type)", "EvictionPolicySchema (const)", + "ExpandViewResult (interface)", "ExpandedViewItem (interface)", "FieldWidgetProps (type)", "FieldWidgetPropsSchema (const)", @@ -3382,6 +3386,7 @@ "ViewItem (type)", "ViewItemNameSchema (const)", "ViewItemSchema (const)", + "ViewKeyCollision (interface)", "ViewKind (type)", "ViewKindSchema (const)", "ViewSchema (const)", @@ -3426,6 +3431,7 @@ "defineView (function)", "defineViewItem (function)", "expandViewContainer (function)", + "expandViewContainerWithDiagnostics (function)", "isAggregatedViewContainer (function)", "pageForm (const)", "reportForm (const)", diff --git a/packages/spec/src/index.ts b/packages/spec/src/index.ts index ea76a79fc9..60736b3a5f 100644 --- a/packages/spec/src/index.ts +++ b/packages/spec/src/index.ts @@ -71,8 +71,8 @@ export type { DefineStackOptions, ComposeStacksOptions, ConflictStrategy, Object export * from './stack.zod'; // DX Helper Functions (re-exported for convenience) -export { defineView, defineForm, defineViewItem, isAggregatedViewContainer, expandViewContainer } from './ui/view.zod'; -export type { ExpandedViewItem } from './ui/view.zod'; +export { defineView, defineForm, defineViewItem, isAggregatedViewContainer, expandViewContainer, expandViewContainerWithDiagnostics } from './ui/view.zod'; +export type { ExpandedViewItem, ViewKeyCollision, ExpandViewResult } from './ui/view.zod'; export { defineApp } from './ui/app.zod'; export { defineFlow } from './automation/flow.zod'; export { defineJob } from './system/job.zod'; diff --git a/packages/spec/src/ui/view.zod.ts b/packages/spec/src/ui/view.zod.ts index 327b6375ed..63f8b12025 100644 --- a/packages/spec/src/ui/view.zod.ts +++ b/packages/spec/src/ui/view.zod.ts @@ -1171,17 +1171,50 @@ function cloneViewConfig(v: any): any { } /** - * Expand an aggregated view container into independent ViewItems. + * A view-key collision detected while expanding one `defineView` container. + * + * List and form views share a single `.` namespace during + * expansion (see {@link expandViewContainer}), and the default `list` + * implicitly claims `.default`. When a later view competes for a name + * already taken, it is renamed (`.` → `._2`) to keep + * the runtime registry key unique. That rename is a silent footgun: references + * to the requested name (form action `target`s, navigation `viewName`s) resolve + * to the OTHER view. The build-time view-ref lint turns each collision into a + * hard error so the author fixes the key instead of shipping a broken reference. + */ +export interface ViewKeyCollision { + /** The `.` name that was already registered by an earlier view. */ + requested: string; + /** The disambiguated name the colliding view was renamed to (e.g. `…_2`). */ + renamedTo: string; + /** Which family the renamed (losing) view belongs to. */ + viewKind: 'list' | 'form'; + /** The raw author-supplied key that collided. */ + key: string; +} + +/** Result of {@link expandViewContainerWithDiagnostics}. */ +export interface ExpandViewResult { + items: ExpandedViewItem[]; + collisions: ViewKeyCollision[]; +} + +/** + * Expand an aggregated view container into independent ViewItems, ALSO reporting + * every name collision that forced a rename. * * List family: `listViews` entries first (keys taken from the author), then the * default `list` — deduped by structural signature so a `listViews.all` that * merely restates `list` collapses into one item. The view matching the - * declared default is flagged `isDefault`. + * declared default is flagged `isDefault`. Form family: `formViews` entries, + * then the default `form`. * - * Form family: `formViews` entries, then the default `form`. + * Collisions are captured at the exact points the shared `used` set forces a + * rename, so the diagnostic can never drift from the expansion it describes. */ -export function expandViewContainer(object: string, container: any): ExpandedViewItem[] { +export function expandViewContainerWithDiagnostics(object: string, container: any): ExpandViewResult { const out: ExpandedViewItem[] = []; + const collisions: ViewKeyCollision[] = []; const used = new Set(); let order = 0; @@ -1191,7 +1224,9 @@ export function expandViewContainer(object: string, container: any): ExpandedVie container.listViews && typeof container.listViews === 'object' ? container.listViews : {}; for (const [k, v] of Object.entries(listViews)) { if (!v || typeof v !== 'object') continue; - const name = uniqueViewName(`${object}.${k}`, used); + const requested = `${object}.${k}`; + const name = uniqueViewName(requested, used); + if (name !== requested) collisions.push({ requested, renamedTo: name, viewKind: 'list', key: k }); listSigToName.set(viewSignature(v), name); out.push({ name, object, viewKind: 'list', label: v.label, config: cloneViewConfig(v), order: order++, scope: 'package' }); } @@ -1203,7 +1238,9 @@ export function expandViewContainer(object: string, container: any): ExpandedVie defaultListName = dup; // already represented by a named listViews entry } else { const key = typeof defaultList.name === 'string' && defaultList.name ? defaultList.name : 'default'; - const name = uniqueViewName(`${object}.${key}`, used); + const requested = `${object}.${key}`; + const name = uniqueViewName(requested, used); + if (name !== requested) collisions.push({ requested, renamedTo: name, viewKind: 'list', key }); out.push({ name, object, viewKind: 'list', label: defaultList.label, config: cloneViewConfig(defaultList), order: order++, scope: 'package' }); defaultListName = name; } @@ -1220,7 +1257,9 @@ export function expandViewContainer(object: string, container: any): ExpandedVie container.formViews && typeof container.formViews === 'object' ? container.formViews : {}; for (const [k, v] of Object.entries(formViews)) { if (!v || typeof v !== 'object') continue; - const name = uniqueViewName(`${object}.${k}`, used); + const requested = `${object}.${k}`; + const name = uniqueViewName(requested, used); + if (name !== requested) collisions.push({ requested, renamedTo: name, viewKind: 'form', key: k }); formSigSeen.add(viewSignature(v)); out.push({ name, object, viewKind: 'form', label: v.label, config: cloneViewConfig(v), order: order++, scope: 'package' }); } @@ -1228,7 +1267,9 @@ export function expandViewContainer(object: string, container: any): ExpandedVie let defaultFormName: string | undefined; if (defaultForm && typeof defaultForm === 'object' && !formSigSeen.has(viewSignature(defaultForm))) { const key = typeof defaultForm.name === 'string' && defaultForm.name ? defaultForm.name : 'form'; - const name = uniqueViewName(`${object}.${key}`, used); + const requested = `${object}.${key}`; + const name = uniqueViewName(requested, used); + if (name !== requested) collisions.push({ requested, renamedTo: name, viewKind: 'form', key }); out.push({ name, object, viewKind: 'form', label: defaultForm.label, config: cloneViewConfig(defaultForm), order: order++, scope: 'package' }); defaultFormName = name; } @@ -1237,7 +1278,18 @@ export function expandViewContainer(object: string, container: any): ExpandedVie if (out[i].name === defaultFormName) out[i].isDefault = true; } - return out; + return { items: out, collisions }; +} + +/** + * Expand an aggregated view container into independent ViewItems. + * + * Thin wrapper over {@link expandViewContainerWithDiagnostics} that discards the + * collision diagnostics — the shape every runtime loader consumes. Behaviour is + * unchanged: colliding names are still renamed so the registry key stays unique. + */ +export function expandViewContainer(object: string, container: any): ExpandedViewItem[] { + return expandViewContainerWithDiagnostics(object, container).items; } /**