Skip to content

Commit 2c8bba5

Browse files
Copilothotlong
andcommitted
fix: remove double-pass defineStack hack, unify config composition flow
- Remove defineStack() validation pass from objectstack.config.ts and apps/console/objectstack.shared.ts that stripped runtime properties (listViews, actions) from objects, requiring a second composeStacks() call to restore them. - Use composed.apps in console shared config instead of manual spreading [...crmApps, ...(todoConfig.apps || []), ...(kitchenSinkConfig.apps || [])] - Use composed.reports instead of ...(crmConfig.reports || []) - Apply CRM navigation patch to composed output instead of pre-composing - Add 2 tests for conflict detection and prefixed naming pattern - Update ROADMAP.md with composeStacks responsibility split documentation Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent eb2cc9d commit 2c8bba5

File tree

4 files changed

+64
-71
lines changed

4 files changed

+64
-71
lines changed

ROADMAP.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1188,7 +1188,11 @@ Plugin architecture refactoring to support true modular development, plugin isol
11881188
- [x] Remove duplicate `mergeActionsIntoObjects()` from root config and console shared config
11891189
- [x] Remove duplicate `mergeViewsIntoObjects()` from root config and console shared config (moved into `composeStacks`)
11901190
- [x] Refactor root `objectstack.config.ts` and `apps/console/objectstack.shared.ts` to use `composeStacks()`
1191-
- [x] Unit tests for `composeStacks()` (13 tests covering merging, dedup, views, actions, cross-stack)
1191+
- [x] Unit tests for `composeStacks()` (15 tests covering merging, dedup, views, actions, cross-stack, conflict detection)
1192+
- [x] Eliminate `defineStack()` double-pass hack — single `composeStacks()` call produces final config with runtime properties (listViews, actions) preserved. `defineStack()` Zod validation stripped these fields, requiring a second `composeStacks` pass to restore them.
1193+
- [x] Use `composed.apps` unified flow in console shared config — replaced manual `[...crmApps, ...(todoConfig.apps || []), ...]` spreading with CRM navigation patch applied to composed output
1194+
- [x] Use `composed.reports` in console shared config — replaced `...(crmConfig.reports || [])` with `...(composed.reports || [])` to include reports from all stacks
1195+
- [x] **composeStacks responsibility split:** `@object-ui/core` composeStacks handles runtime mapping (merging views→objects listViews, actions→objects) while `@objectstack/spec` composeStacks handles protocol-level composition (broader field concatenation, manifest selection, i18n). ObjectUI apps use the core version for single-pass config with runtime properties preserved.
11921196

11931197
**Phase 2 — Dynamic Plugin Loading (Planned)**
11941198
- [ ] Hot-reload / lazy loading of plugins for development

apps/console/objectstack.shared.ts

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { defineStack } from '@objectstack/spec';
21
import type { ObjectStackDefinition } from '@objectstack/spec';
32
import { composeStacks } from '@object-ui/core';
43
import crmConfigImport from '@object-ui/example-crm/objectstack.config';
@@ -18,33 +17,31 @@ const crmConfig = resolveDefault<ObjectStackDefinition>(crmConfigImport);
1817
const todoConfig = resolveDefault<ObjectStackDefinition>(todoConfigImport);
1918
const kitchenSinkConfig = resolveDefault<ObjectStackDefinition>(kitchenSinkConfigImport);
2019

21-
// Patch CRM App Navigation to include Report using a supported navigation type
22-
// (type: 'url' passes schema validation while still routing correctly via React Router)
23-
const crmApps = crmConfig.apps ? JSON.parse(JSON.stringify(crmConfig.apps)) : [];
24-
if (crmApps.length > 0) {
25-
const crmApp = crmApps[0];
26-
if (crmApp && crmApp.navigation) {
27-
// Insert report after dashboard
28-
const dashboardIdx = crmApp.navigation.findIndex((n: any) => n.id === 'nav_dashboard');
29-
const insertIdx = dashboardIdx !== -1 ? dashboardIdx + 1 : 0;
30-
crmApp.navigation.splice(insertIdx, 0, {
31-
id: 'nav_sales_report',
32-
type: 'url',
33-
url: '/apps/crm_app/report/sales_performance_q1',
34-
label: 'Sales Report',
35-
icon: 'file-bar-chart'
36-
});
37-
}
38-
}
39-
40-
// Compose all example stacks into a single merged definition.
41-
// composeStacks handles object deduplication (override), views→objects mapping,
42-
// and actions→objects assignment via objectName.
20+
// Single-pass composition: composeStacks handles object deduplication (override),
21+
// views→objects mapping, and actions→objects assignment via objectName.
22+
// No defineStack() validation pass — it would strip runtime properties (listViews,
23+
// actions) from objects, requiring a double-pass hack to restore them.
4324
const composed = composeStacks(
4425
[crmConfig, todoConfig, kitchenSinkConfig] as Record<string, any>[],
4526
{ objectConflict: 'override' },
4627
);
4728

29+
// Patch CRM App Navigation to include Report using a supported navigation type
30+
// (type: 'url' passes schema validation while still routing correctly via React Router)
31+
const apps = JSON.parse(JSON.stringify(composed.apps || []));
32+
const crmApp = apps.find((a: any) => a.name === 'crm_app');
33+
if (crmApp?.navigation) {
34+
const dashboardIdx = crmApp.navigation.findIndex((n: any) => n.id === 'nav_dashboard');
35+
const insertIdx = dashboardIdx !== -1 ? dashboardIdx + 1 : 0;
36+
crmApp.navigation.splice(insertIdx, 0, {
37+
id: 'nav_sales_report',
38+
type: 'url',
39+
url: '/apps/crm_app/report/sales_performance_q1',
40+
label: 'Sales Report',
41+
icon: 'file-bar-chart'
42+
});
43+
}
44+
4845
export const sharedConfig = {
4946
// ============================================================================
5047
// Project Metadata
@@ -58,15 +55,11 @@ export const sharedConfig = {
5855
// Merged Stack Configuration (CRM + Todo + Kitchen Sink)
5956
// ============================================================================
6057
objects: composed.objects,
61-
apps: [
62-
...crmApps,
63-
...(todoConfig.apps || []),
64-
...(kitchenSinkConfig.apps || []),
65-
],
58+
apps,
6659
dashboards: composed.dashboards,
6760
reports: [
68-
...(crmConfig.reports || []),
69-
// Manually added report since CRM config validation prevents it
61+
...(composed.reports || []),
62+
// Console-specific report not in any example stack
7063
{
7164
name: 'sales_performance_q1',
7265
label: 'Q1 Sales Performance',
@@ -99,17 +92,4 @@ export const sharedConfig = {
9992
]
10093
};
10194

102-
const allConfigs = [crmConfig, todoConfig, kitchenSinkConfig];
103-
104-
// defineStack() validates the config but strips non-standard properties like
105-
// listViews and actions from objects. A second composeStacks pass restores
106-
// these runtime properties onto the validated objects. This double-pass is
107-
// necessary because defineStack's Zod schema doesn't preserve custom fields.
108-
const validated = defineStack(sharedConfig as Parameters<typeof defineStack>[0]);
109-
export default {
110-
...validated,
111-
objects: composeStacks([
112-
{ objects: validated.objects || [] },
113-
...allConfigs.map((cfg: any) => ({ views: cfg.views || [], actions: cfg.actions || [] })),
114-
]).objects,
115-
};
95+
export default sharedConfig;

objectstack.config.ts

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* `kernel.use(new CRMPlugin())`. In the dev workspace, we collect their
1515
* configs via `getConfig()` and merge them with `composeStacks()`.
1616
*/
17-
import { defineStack } from '@objectstack/spec';
1817
import { AppPlugin, DriverPlugin } from '@objectstack/runtime';
1918
import { ObjectQLPlugin } from '@objectstack/objectql';
2019
import { InMemoryDriver } from '@objectstack/driver-memory';
@@ -34,14 +33,14 @@ const allConfigs = plugins.map((p) => {
3433
return (raw as any).default || raw;
3534
});
3635

37-
// Compose all plugin configs into a single stack definition.
38-
// composeStacks handles object deduplication, views→objects mapping,
39-
// and actions→objects assignment via objectName.
36+
// Single-pass composition: composeStacks handles object deduplication,
37+
// views→objects mapping, and actions→objects assignment via objectName.
38+
// No defineStack() validation pass needed — it would strip runtime properties
39+
// (listViews, actions) from objects, requiring a second merge pass to restore them.
4040
const composed = composeStacks(allConfigs, { objectConflict: 'override' });
4141

42-
// Validate via defineStack, then re-apply runtime properties (listViews, actions)
43-
// that defineStack strips during validation.
44-
const mergedApp = defineStack({
42+
const mergedApp = {
43+
...composed,
4544
manifest: {
4645
id: 'dev-workspace',
4746
name: 'dev_workspace',
@@ -50,24 +49,6 @@ const mergedApp = defineStack({
5049
type: 'app',
5150
data: composed.manifest.data,
5251
},
53-
objects: composed.objects,
54-
views: composed.views,
55-
apps: composed.apps,
56-
dashboards: composed.dashboards,
57-
reports: composed.reports,
58-
pages: composed.pages,
59-
} as any);
60-
61-
// defineStack() validates the config but strips non-standard properties like
62-
// listViews and actions from objects. A second composeStacks pass restores
63-
// these runtime properties onto the validated objects. This double-pass is
64-
// necessary because defineStack's Zod schema doesn't preserve custom fields.
65-
const mergedAppWithViews = {
66-
...mergedApp,
67-
objects: composeStacks([
68-
{ objects: mergedApp.objects || [] },
69-
...allConfigs.map((cfg: any) => ({ views: cfg.views || [], actions: cfg.actions || [] })),
70-
]).objects,
7152
};
7253

7354
// Export only plugins — no top-level objects/manifest/apps.
@@ -77,7 +58,7 @@ export default {
7758
plugins: [
7859
new ObjectQLPlugin(),
7960
new DriverPlugin(new InMemoryDriver()),
80-
new AppPlugin(mergedAppWithViews),
61+
new AppPlugin(mergedApp),
8162
new HonoServerPlugin({ port: 3000 }),
8263
new ConsolePlugin(),
8364
],

packages/core/src/utils/__tests__/compose-stacks.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,4 +281,32 @@ describe('composeStacks', () => {
281281
// Manifest data merged
282282
expect(result.manifest.data).toHaveLength(2);
283283
});
284+
285+
it('should detect conflicting object names with objectConflict: "error"', () => {
286+
// Simulates a scenario where two plugins define 'account' without prefixing
287+
const crm = {
288+
objects: [{ name: 'account', label: 'CRM Account', fields: {} }],
289+
};
290+
const ks = {
291+
objects: [{ name: 'account', label: 'KS Account', fields: {} }],
292+
};
293+
294+
expect(() => composeStacks([crm, ks], { objectConflict: 'error' })).toThrow(
295+
'duplicate object name "account"'
296+
);
297+
});
298+
299+
it('should allow prefixed object names to coexist without conflict', () => {
300+
// After renaming: CRM keeps 'account', Kitchen Sink uses 'ks_account'
301+
const crm = {
302+
objects: [{ name: 'account', label: 'CRM Account', fields: {} }],
303+
};
304+
const ks = {
305+
objects: [{ name: 'ks_account', label: 'KS Account', fields: {} }],
306+
};
307+
308+
const result = composeStacks([crm, ks], { objectConflict: 'error' });
309+
expect(result.objects).toHaveLength(2);
310+
expect(result.objects.map((o: any) => o.name).sort()).toEqual(['account', 'ks_account']);
311+
});
284312
});

0 commit comments

Comments
 (0)