Skip to content

Commit df18ae9

Browse files
os-zhuangCopilot
andcommitted
Fix HMR view-name resolution + split watch from artifactWatch
The blocked verify-hmr-visual smoke test (edit a view in dev mode → expect API to return fresh value < 1s) was failing for two reasons. 1. View bundles in the compiled artifact have no top-level \`name\` — their identity is the target object encoded under \`list.data.object\` (same convention used by ObjectQL.SchemaRegistry.resolveMetadataItemName). MetadataPlugin._parseAndRegisterArtifact had a naive so MetadataService.get('view', X) always returned undefined and the read path fell through to the boot-only SchemaRegistry copy. Derive the registration key from \`list.data.object\` (or \`form.data.object\`) when no top-level \`name\` is present. 2. Enabling MetadataPlugin's \`watch\` to pick up dist/objectstack.json changes also turned on NodeMetadataManager's recursive source-file scan, which polling-scanned the entire project root (incl. node_modules) and either hung startup or hit EMFILE on macOS. Split into two flags: - \`watch\`: source-file scanner (default false; opt-in) - \`artifactWatch\`: cheap single-file watcher on the compiled artifact (auto-enabled in dev by StandaloneStack) Also pre-emptively switch the CLI dev watcher and NodeMetadataManager to chokidar polling — native fs.watch hits EMFILE on busy macOS hosts where parallel node processes saturate the FD pool. Regression test in packages/metadata/src/plugin.test.ts asserts that a view artifact item with no top-level \`name\` lands in MetadataManager under its target-object key. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 96ad4df commit df18ae9

8 files changed

Lines changed: 163 additions & 12 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
'@objectstack/metadata': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
Fix dev-mode HMR data-reload for view metadata.
7+
8+
`MetadataPlugin._parseAndRegisterArtifact` previously required a top-level
9+
`name` on every artifact item and silently skipped those without one.
10+
View bundles in the compiled artifact carry no top-level `name` (their
11+
identity is the target object, encoded under `list.data.object` /
12+
`form.data.object` — same pattern used by `ObjectQL.SchemaRegistry`'s
13+
`resolveMetadataItemName`). As a result, artifact-loaded views never
14+
reached `MetadataManager`, and HMR file pushes never affected the read
15+
path: API responses kept returning the boot-time `SchemaRegistry` copy.
16+
17+
This change derives the registration key from `list.data.object` (or
18+
`form.data.object`) when no top-level `name` is present, mirroring the
19+
ObjectQL convention.
20+
21+
Also splits the `MetadataPlugin` watch flag into two independent
22+
options so dev mode can enable artifact-file HMR without paying the
23+
cost of the source-file scanner:
24+
25+
- `watch` — controls `NodeMetadataManager`'s recursive source scan
26+
(default `false`; turning it on in artifact mode would polling-scan
27+
the entire project root including `node_modules`).
28+
- `artifactWatch` — controls the cheap single-file polling watcher on
29+
the compiled artifact (`dist/objectstack.json`). The standalone stack
30+
enables this automatically when `NODE_ENV !== 'production'`.

examples/app-crm/dist/objectstack.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14125,7 +14125,7 @@
1412514125
"listViews": {
1412614126
"case_workflow": {
1412714127
"name": "case_workflow",
14128-
"label": "Service Workflow",
14128+
"label": "Service Workflow (HMR-LIVE)",
1412914129
"type": "kanban",
1413014130
"data": {
1413114131
"provider": "object",

packages/cli/src/commands/dev.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,13 @@ export default class Dev extends Command {
237237
],
238238
ignoreInitial: true,
239239
persistent: true,
240+
// Use polling to avoid `fs.watch` EMFILE on macOS when other
241+
// long-running node processes (VS Code, parallel dev servers)
242+
// saturate the native file-descriptor pool. Polling at 750ms
243+
// is fast enough for human-perceived HMR.
244+
usePolling: true,
245+
interval: 750,
246+
binaryInterval: 1500,
240247
});
241248

242249
let timer: ReturnType<typeof setTimeout> | null = null;

packages/metadata/src/node-metadata-manager.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ export class NodeMetadataManager extends MetadataManager {
5959
ignored,
6060
persistent,
6161
ignoreInitial: true,
62+
// Use polling to avoid `fs.watch` EMFILE on macOS / busy dev hosts.
63+
// Recursive watch over a project root would otherwise wire native
64+
// watches across the entire tree, easily exhausting the FD pool.
65+
usePolling: true,
66+
interval: 1000,
67+
binaryInterval: 2000,
6268
});
6369

6470
this.watcher.on('add', async (filePath) => {

packages/metadata/src/plugin.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,56 @@ describe('MetadataPlugin — bootstrap × watch coupling (D2)', () => {
5757
expect((mgr as any).watcher).toBeUndefined();
5858
});
5959
});
60+
61+
// ─────────────────────────────────────────────────────────────────────────
62+
// PR-10e regression: artifact view items have no top-level `name`. Their
63+
// identity is the target object (encoded in `list.data.object` /
64+
// `form.data.object`). When `_parseAndRegisterArtifact` consumes a
65+
// compiled artifact it must derive the view name from the inner data
66+
// source — otherwise views are silently SKIPPED and reads through
67+
// `metadataService.get('view', <object>)` return undefined, falling
68+
// back to the boot-time SchemaRegistry copy and breaking HMR data
69+
// reload.
70+
// ─────────────────────────────────────────────────────────────────────────
71+
describe('MetadataPlugin._parseAndRegisterArtifact — view name resolution (PR-10e)', () => {
72+
it('registers view items by their target object even when top-level `name` is absent', async () => {
73+
const plugin = new MetadataPlugin({
74+
watch: false,
75+
config: { bootstrap: 'eager' },
76+
projectId: 'proj_test',
77+
});
78+
const mgr = (plugin as any).manager as NodeMetadataManager;
79+
80+
const fakeCtx = {
81+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
82+
} as any;
83+
84+
const artifact = {
85+
id: 'com.example.test',
86+
name: 'test',
87+
version: '0.0.0',
88+
type: 'app',
89+
scope: 'app',
90+
namespace: 'test',
91+
defaultDatasource: 'memory',
92+
views: [
93+
{
94+
// intentionally NO top-level name — mirrors compiled artifact shape
95+
list: { name: 'all_case', label: 'All Cases', type: 'grid',
96+
data: { provider: 'object', object: 'case' }, columns: [] },
97+
listViews: {
98+
case_workflow: { name: 'case_workflow', label: 'Service Workflow', type: 'kanban',
99+
data: { provider: 'object', object: 'case' }, columns: [] },
100+
},
101+
},
102+
],
103+
};
104+
105+
await (plugin as any)._parseAndRegisterArtifact(fakeCtx, artifact, 'test-artifact');
106+
107+
const registered = await mgr.get('view', 'case');
108+
expect(registered).toBeDefined();
109+
const label = (registered as any)?.listViews?.case_workflow?.label;
110+
expect(label).toBe('Service Workflow');
111+
});
112+
});

packages/metadata/src/plugin.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,30 @@ const ARTIFACT_FIELD_TO_TYPE: Record<string, string> = {
6262

6363
export interface MetadataPluginOptions {
6464
rootDir?: string;
65+
/**
66+
* When `true`, NodeMetadataManager scans `rootDir` for source-file metadata
67+
* (yaml/json/ts/js loaders) AND attaches a chokidar watcher to react to
68+
* filesystem changes. In **artifact-mode** (this is the normal path when
69+
* a `defineStack()` config is compiled into `dist/objectstack.json`) this
70+
* filesystem scan is redundant and expensive — leave `watch: false`.
71+
*
72+
* The artifact-file HMR watcher is controlled separately by
73+
* {@link artifactWatch} so that the cheap, single-file polling watcher
74+
* can be enabled in dev without paying the cost of scanning the entire
75+
* project root.
76+
*
77+
* Default: `false` (post PR-10e — was previously `true`).
78+
*/
6579
watch?: boolean;
80+
/**
81+
* When `true` AND `artifactSource.mode === 'local-file'`, attach a
82+
* polling chokidar watcher to the artifact file so the server reloads
83+
* metadata when the CLI recompiles `dist/objectstack.json` in dev mode.
84+
* Independent of {@link watch} (which controls the source-file scanner).
85+
*
86+
* Default: `true` when `artifactSource` is set, otherwise `false`.
87+
*/
88+
artifactWatch?: boolean;
6689
config?: Partial<MetadataPluginConfig>;
6790
/** Organization ID for metadata-scoped consumers; MetadataPlugin itself does not persist runtime metadata. */
6891
organizationId?: string;
@@ -297,7 +320,7 @@ export class MetadataPlugin implements Plugin {
297320
}
298321
});
299322

300-
// ── ADR-0008 PR-8: server-side artifact-file watcher ────
323+
// ── ADR-0008 PR-8 / PR-10e: server-side artifact-file watcher ──
301324
//
302325
// When running in local-file artifact mode (e.g. `os dev`
303326
// serving from `dist/objectstack.json`), watch the
@@ -306,8 +329,15 @@ export class MetadataPlugin implements Plugin {
306329
// POST endpoint. The POST route stays available for
307330
// external trigger sources (cloud webhook, git hook,
308331
// ad-hoc curl) but is no longer the only signal.
332+
//
333+
// Gated on `artifactWatch` (NOT `watch` — the latter
334+
// controls the source-file scanner which is redundant in
335+
// artifact mode). Default: on when artifactSource is
336+
// present, off otherwise.
309337
const src = this.options.artifactSource;
310-
if (src?.mode === 'local-file' && this.options.watch !== false && !/^https?:\/\//i.test(src.path)) {
338+
const wantArtifactWatch = this.options.artifactWatch
339+
?? (src?.mode === 'local-file');
340+
if (src?.mode === 'local-file' && wantArtifactWatch && !/^https?:\/\//i.test(src.path)) {
311341
try {
312342
const { watch: chokidarWatch } = await import('chokidar');
313343
const w = chokidarWatch(src.path, {
@@ -449,7 +479,26 @@ export class MetadataPlugin implements Plugin {
449479
const items = (metadata as any)[field];
450480
if (!Array.isArray(items) || items.length === 0) continue;
451481
for (const item of items) {
452-
const name = (item as any)?.name;
482+
// Most metadata items carry a top-level `name`. The `View`
483+
// container (UI namespace) is an exception: it has no own
484+
// `name` — its identity is the target object, encoded under
485+
// `list.data.object` (or `form.data.object`). Mirror the
486+
// resolution used by `ObjectQL.SchemaRegistry` so that
487+
// artifact-loaded views land in `MetadataManager` under the
488+
// SAME key reads expect (`metadataService.get('view', <object>)`).
489+
// Without this, HMR pushes views into the registry only via
490+
// AppPlugin's `manifest.register`, which targets the
491+
// boot-only SchemaRegistry cache and is never refreshed on
492+
// file edits — leaving MetadataService empty for view/* and
493+
// forcing all reads to return the stale boot copy.
494+
let name = (item as any)?.name;
495+
if (!name) {
496+
if (metaType === 'view') {
497+
name =
498+
(item as any)?.list?.data?.object
499+
?? (item as any)?.form?.data?.object;
500+
}
501+
}
453502
if (!name) continue;
454503
if (manifestPackageId && (item as any)._packageId === undefined) {
455504
(item as any)._packageId = manifestPackageId;

packages/metadata/vitest.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export default defineConfig({
99
'@objectstack/core': path.resolve(__dirname, '../core/src/index.ts'),
1010
'@objectstack/driver-memory': path.resolve(__dirname, '../plugins/driver-memory/src/index.ts'),
1111
'@objectstack/spec/api': path.resolve(__dirname, '../spec/src/api/index.ts'),
12+
'@objectstack/spec/cloud': path.resolve(__dirname, '../spec/src/cloud/index.ts'),
1213
'@objectstack/spec/contracts': path.resolve(__dirname, '../spec/src/contracts/index.ts'),
1314
'@objectstack/spec/data': path.resolve(__dirname, '../spec/src/data/index.ts'),
1415
'@objectstack/spec/kernel': path.resolve(__dirname, '../spec/src/kernel/index.ts'),

packages/runtime/src/standalone-stack.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,19 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro
163163
const plugins: any[] = [
164164
driverPlugin,
165165
new MetadataPlugin({
166-
// Enable the artifact-file watcher in development so edits to
167-
// `*.view.ts` / `*.flow.ts` etc. (which the CLI dev-mode watcher
168-
// recompiles into `dist/objectstack.json`) are picked up by the
169-
// running server WITHOUT requiring a manual restart or HMR POST.
170-
// In production (`NODE_ENV=production`), the artifact is static
171-
// and watching is unnecessary — keep the historical default-off
172-
// behavior to avoid the chokidar process cost.
173-
watch: process.env.NODE_ENV !== 'production',
166+
// Source-file scanner OFF — declarative metadata is loaded
167+
// from the compiled artifact, not from yaml/json files on
168+
// disk. Scanning would also recursively watch the project
169+
// root (incl. node_modules), which is expensive and prone
170+
// to EMFILE.
171+
watch: false,
172+
// Artifact-file HMR ON in non-production so edits to
173+
// `*.view.ts` / `*.flow.ts` (which the CLI dev-mode watcher
174+
// recompiles into `dist/objectstack.json`) are picked up by
175+
// the running server WITHOUT requiring a manual restart.
176+
// Uses polling under the hood (see plugin.ts) to avoid
177+
// `fs.watch` EMFILE on macOS / busy dev hosts.
178+
artifactWatch: process.env.NODE_ENV !== 'production',
174179
projectId,
175180
artifactSource: { mode: 'local-file', path: artifactPath },
176181
}),

0 commit comments

Comments
 (0)