Skip to content

Commit f014be8

Browse files
committed
fix(cli): report the named export the config's default export already declares, instead of dropping it silently
`loadConfig()` merges every named export of `objectstack.config.ts` onto the default-exported stack as a top-level key. A name the default already carried was skipped by `if (key === 'default' || key in merged) continue` — the build exited 0, the artifact carried the default's value, and nothing was written at any level, so authored content vanished on the success path. The drop itself is correct and stays: one key, one value. What changes is that it is now REPORTED — on stderr, from the loader, so all twelve commands that load a config carry it and a `--json` run's stdout stays a single parseable document — and recorded structurally on `LoadedConfig.shadowedNamedExports`. Advisory, never fatal: the stack that comes out is valid, it is merely missing what the shadowed export carried. That disposition is this package's own for the class (#3786's undeclared authoring keys, #4095's orphaned runtime members), not a fresh judgement. Sweeping the arm for other silent shapes found a second one in the same expression: `key in merged` walks the PROTOTYPE chain, so `Object.prototype`'s members answered true for a default export carrying no such key. An `export const toString = …` was skipped by the collision arm and therefore never reached the strict parse that refuses an undeclared stack key by name — the loud refusal, silently turned off by the spelling of the key. The test is now `Object.prototype.hasOwnProperty.call`, which both closes that hole and is what makes the new diagnostic truthful: without it the loader would report `toString` as shadowed by a default export that declares nothing of the sort. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 32be735 commit f014be8

5 files changed

Lines changed: 404 additions & 10 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
fix(cli)!: a named export the config's default export already declares is reported instead of silently dropped (#18419)
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) no metadata key is retired, renamed or given a new meaning here, no stored `sys_metadata` document changes, and the only authored construct whose treatment moves is a NAMED EXPORT of `objectstack.config.ts` that was already inert — it was skipped by the merge and reached no artifact. `os migrate meta` has nothing it could rewrite, so there is no ledger entry for this to be missing. -->
8+
9+
`objectstack.config.ts` is loaded as a module: `loadConfig()` takes the default export as the base and merges every named export onto it as a top-level stack key. A named export whose name the default export **already carries** loses — the default's value wins — and until now it lost in complete silence. `os build` exited 0, the artifact carried the default's value, and nothing was written at any level:
10+
11+
```ts
12+
export default defineStack({ manifest, objects: [Task] });
13+
export const objects = [Task, Invoice]; // Invoice never reached the artifact
14+
```
15+
16+
The loader now says so on stderr, names every shadowed key, and states the rule and the remedy. It is an **advisory, not a refusal** — the stack that comes out is valid, it is merely missing what the shadowed export carried — which is the disposition this package already gives the same failure class (`#3786`'s undeclared authoring keys are "advisory, never fatal"; `#4095`'s orphaned runtime members are "reported rather than dropped"). It goes to stderr rather than stdout because `loadConfig()` is handed no `--json` flag and twelve commands call it, so a `--json` run's stdout stays a single parseable document. `LoadedConfig.shadowedNamedExports` carries the same names structurally.
17+
18+
**BREAKING** in the accept-set sense, landing in the launch window as `minor` (the lockstep convention: `major` is refused by `check-changeset-no-major`, and breaking-ness is carried by this banner plus the ADR-0087 disposition above): the collision test now reads **own keys only**. `key in merged` walked the prototype chain, so every `Object.prototype` member — `toString`, `valueOf`, `constructor`, `hasOwnProperty`, `propertyIsEnumerable`, `toLocaleString`, `isPrototypeOf` — was treated as a key the default export "already carries" when the default carries no such key at all. Such an export was skipped by the merge and therefore never reached the strict parse that refuses an undeclared stack key by name, so `export const toString = …` beside a valid stack built green while `export const collectPackageDirs = …` was refused. That hole is closed: those names now merge like any other and are refused by name, the same sentence every other undeclared helper export has always got.
19+
20+
Nobody's metadata or stored data changes. A config affected by the narrowing was already shipping that export's value nowhere; what changes is that the build now says so instead of exiting 0. Move the helper into a sibling module and import it, which is what the config-authoring docs have always prescribed for a helper exported beside the stack.

‎content/docs/deployment/cli.mdx‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,10 +2063,21 @@ recognisable rather than as patterns to use:
20632063
|:---|:---|
20642064
| not a declared stack key | the build **fails**, naming the key (above) |
20652065
| a declared stack key the default export does **not** carry | merged in and **accepted** — the `onEnable` / `functions` path |
2066-
| a key the default export **already** carries | dropped **silently**; the build exits 0 and the exported value is never read |
2066+
| a key the default export **already** carries | the default's value wins and the exported one is dropped — the build still exits 0, and the drop is **reported on stderr** |
20672067
20682068
The last row is why every stack key belongs inside `defineStack()`: a second
2069-
copy beside it is not a second declaration.
2069+
copy beside it is not a second declaration, it is a value nothing reads.
2070+
2071+
```console
2072+
$ printf '\nexport const objects = [myExtraObject];\n' >> objectstack.config.ts
2073+
$ os build
2074+
⚠ `objects` is a named export that was DROPPED — the default-exported stack already declares that key
2075+
```
2076+
2077+
The advisory goes to **stderr**, so it reaches a `--json` run's operator without
2078+
putting anything but the envelope on stdout. It does not fail the build: the
2079+
stack it produces is valid, it is simply missing what the shadowed export
2080+
carried.
20702081
20712082
### Config File Auto-Detection
20722083
Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* A named export the default-exported stack already declares is DROPPED — and
5+
* the drop is now reported instead of silent (#18419).
6+
*
7+
* ── The finding ──────────────────────────────────────────────────────────
8+
*
9+
* `loadConfig()` merges every named export of `objectstack.config.ts` onto the
10+
* default export as a top-level stack key. A name the default already carries
11+
* was skipped by `if (key === 'default' || key in merged) continue`, so:
12+
*
13+
* export default defineStack({ manifest, objects: [] });
14+
* export const objects = [oneRow]; // <- never reaches anything
15+
*
16+
* built to exit 0, wrote an artifact whose `objects` is the default's `[]`, and
17+
* logged NOTHING at any level. Authored content vanished on the success path.
18+
*
19+
* ── The positive control this file carries ───────────────────────────────
20+
*
21+
* The same channel is LOUD for a named export the stack schema does not
22+
* declare: `export const ProbeNamedExport = [1,2,3]` is merged in, refused by
23+
* the strict parse and named in the message (#18171, pinned next door in
24+
* `config-named-export-rule.test.ts`). So silence was specific to this one arm
25+
* rather than a property of the loader — which is why the first two pins below
26+
* assert the UNCHANGED rows. A pin that only proved "a warning appears" could
27+
* not tell a repaired loader from one that warns about everything.
28+
*
29+
* ── The second arm, measured while sweeping for the first ────────────────
30+
*
31+
* `key in merged` walks the PROTOTYPE chain, so `Object.prototype`'s members
32+
* answered true for a default export that carries no such key at all. An
33+
* `export const toString = …` was therefore skipped by the collision arm and
34+
* never reached the strict parse that would have refused it by name — the loud
35+
* refusal above, silently turned off by the spelling of the key. `loadConfig`
36+
* now tests own keys only, so that row rejoins the control.
37+
*
38+
* ── Disposition: advisory, not refusal ───────────────────────────────────
39+
*
40+
* Read off this package's own repairs of this class — #3786's undeclared
41+
* authoring keys ("Advisory, never fatal") and #4095's orphaned runtime members
42+
* ("reported rather than dropped") — and recorded in full on
43+
* {@link shadowedNamedExportWarning}. The stack that comes out is valid; it is
44+
* merely missing what the shadowed export carried, so the run continues and the
45+
* author is told. The accept set therefore moves for the prototype-chain row
46+
* only, and in the narrowing direction.
47+
*/
48+
49+
import { describe, it, expect, afterAll, vi } from 'vitest';
50+
import fs from 'node:fs';
51+
import path from 'node:path';
52+
import { fileURLToPath } from 'node:url';
53+
import { ObjectStackDefinitionSchema } from '@objectstack/spec';
54+
55+
import { loadConfig, shadowedNamedExportWarning } from './config.js';
56+
57+
const HERE = path.dirname(fileURLToPath(import.meta.url));
58+
/** `packages/cli/tmp` — throwaway projects, the placement sibling suites use. */
59+
const TMP_ROOT = path.resolve(HERE, '..', '..', 'tmp');
60+
61+
/** The card's own probe export — a name the stack schema does not declare. */
62+
const PROBE = 'ProbeNamedExport';
63+
64+
const MANIFEST = `{
65+
id: 'com.example.probe',
66+
namespace: 'probe',
67+
version: '0.1.0',
68+
type: 'app',
69+
name: 'Probe',
70+
engines: { protocol: '^17' },
71+
}`;
72+
73+
const roots: string[] = [];
74+
afterAll(() => {
75+
for (const dir of roots) fs.rmSync(dir, { recursive: true, force: true });
76+
});
77+
78+
function writeConfig(tag: string, body: string): string {
79+
fs.mkdirSync(TMP_ROOT, { recursive: true });
80+
const dir = fs.mkdtempSync(path.join(TMP_ROOT, `shadowed-${tag}-`));
81+
roots.push(dir);
82+
const file = path.join(dir, 'objectstack.config.ts');
83+
fs.writeFileSync(file, body);
84+
return file;
85+
}
86+
87+
/** Load `body`, capturing whatever the loader wrote to each stream. */
88+
async function loadCapturing(tag: string, body: string) {
89+
const err: string[] = [];
90+
const out: string[] = [];
91+
const errSpy = vi.spyOn(console, 'error').mockImplementation((...a) => { err.push(a.join(' ')); });
92+
const outSpy = vi.spyOn(console, 'log').mockImplementation((...a) => { out.push(a.join(' ')); });
93+
try {
94+
const loaded = await loadConfig(writeConfig(tag, body));
95+
return { ...loaded, stderr: err.join('\n'), stdout: out.join('\n') };
96+
} finally {
97+
errSpy.mockRestore();
98+
outSpy.mockRestore();
99+
}
100+
}
101+
102+
describe('#18419 — a named export the default already declares is dropped, and said so', () => {
103+
it('CONTROL: an undeclared named export is still refused by name, and is not a "drop"', async () => {
104+
const loaded = await loadCapturing('control', `import { defineStack } from '@objectstack/spec';
105+
106+
export default defineStack({
107+
manifest: ${MANIFEST},
108+
});
109+
110+
export const ${PROBE} = [1, 2, 3];
111+
`);
112+
113+
// Merged, therefore reaches the parse, therefore refused — the property
114+
// #18171 pinned and this change must not spend.
115+
expect(loaded.namedExports).toEqual([PROBE]);
116+
expect(loaded.shadowedNamedExports).toEqual([]);
117+
const result = ObjectStackDefinitionSchema.safeParse(loaded.config);
118+
expect(result.success).toBe(false);
119+
if (result.success) return;
120+
const unrecognized = result.error.issues.filter((i) => i.code === 'unrecognized_keys');
121+
expect((unrecognized[0] as unknown as { keys: string[] }).keys).toEqual([PROBE]);
122+
123+
// Nothing was dropped, so nothing is announced. "Reported" has to be
124+
// distinguishable from "always reported".
125+
expect(loaded.stderr).not.toContain('DROPPED');
126+
}, 60_000);
127+
128+
it('CONTROL: a declared key the default does NOT carry is still merged, silently and legally', async () => {
129+
const loaded = await loadCapturing('accepted', `import { defineStack } from '@objectstack/spec';
130+
131+
export default defineStack({
132+
manifest: ${MANIFEST},
133+
});
134+
135+
export const objects = [];
136+
`);
137+
138+
expect(loaded.namedExports).toEqual(['objects']);
139+
expect(loaded.shadowedNamedExports).toEqual([]);
140+
expect(ObjectStackDefinitionSchema.safeParse(loaded.config).success).toBe(true);
141+
expect(loaded.stderr).toBe('');
142+
}, 60_000);
143+
144+
it('the collision is RECORDED and ANNOUNCED — the silence this card is about', async () => {
145+
const loaded = await loadCapturing('collision', `import { defineStack } from '@objectstack/spec';
146+
147+
export default defineStack({
148+
manifest: ${MANIFEST},
149+
objects: [],
150+
});
151+
152+
export const objects = [{ name: 'probe_row', label: 'Probe Row', fields: { name: { type: 'text', label: 'Name' } } }];
153+
`);
154+
155+
// The drop itself is unchanged — the default's value still wins…
156+
expect(loaded.config.objects).toEqual([]);
157+
expect(loaded.namedExports).toEqual([]);
158+
// …and it is no longer invisible.
159+
expect(loaded.shadowedNamedExports).toEqual(['objects']);
160+
expect(loaded.stderr).toContain('objects');
161+
expect(loaded.stderr).toContain('DROPPED');
162+
// The rule and the remedy, not just the fact.
163+
expect(loaded.stderr).toContain('loaded as a MODULE');
164+
expect(loaded.stderr).toContain('defineStack');
165+
166+
// Advisory, never fatal: the stack that comes out is still valid.
167+
expect(ObjectStackDefinitionSchema.safeParse(loaded.config).success).toBe(true);
168+
}, 60_000);
169+
170+
it('…including on `functions` — the runtime member an app really does author', async () => {
171+
const loaded = await loadCapturing('functions', `import { defineStack } from '@objectstack/spec';
172+
173+
export default defineStack({
174+
manifest: ${MANIFEST},
175+
functions: { fromDefault: () => 'default' },
176+
});
177+
178+
export const functions = { fromNamedExport: () => 'named' };
179+
`);
180+
181+
expect(loaded.shadowedNamedExports).toEqual(['functions']);
182+
expect(Object.keys(loaded.config.functions)).toEqual(['fromDefault']);
183+
// The handler that vanished is named, because that is the one the author
184+
// has to go looking for.
185+
expect(loaded.stderr).toContain('functions');
186+
}, 60_000);
187+
188+
it('a name that is only on Object.prototype is NOT a collision — it rejoins the control', async () => {
189+
const loaded = await loadCapturing('proto', `import { defineStack } from '@objectstack/spec';
190+
191+
export default defineStack({
192+
manifest: ${MANIFEST},
193+
});
194+
195+
export const toString = [1, 2, 3];
196+
`);
197+
198+
// The default export carries no `toString` of its own, so nothing shadows
199+
// this — it is an undeclared stack key like any other, and goes the loud way.
200+
expect(loaded.shadowedNamedExports).toEqual([]);
201+
expect(loaded.namedExports).toEqual(['toString']);
202+
const result = ObjectStackDefinitionSchema.safeParse(loaded.config);
203+
expect(result.success).toBe(false);
204+
if (result.success) return;
205+
const unrecognized = result.error.issues.filter((i) => i.code === 'unrecognized_keys');
206+
expect((unrecognized[0] as unknown as { keys: string[] }).keys).toContain('toString');
207+
}, 60_000);
208+
209+
it('the advisory goes to stderr only — a --json run keeps stdout parseable', async () => {
210+
const loaded = await loadCapturing('streams', `import { defineStack } from '@objectstack/spec';
211+
212+
export default defineStack({
213+
manifest: ${MANIFEST},
214+
objects: [],
215+
});
216+
217+
export const objects = [{ name: 'probe_row', label: 'Probe Row', fields: { name: { type: 'text', label: 'Name' } } }];
218+
`);
219+
220+
expect(loaded.stderr).toContain('DROPPED');
221+
// `loadConfig` is handed no `--json` flag, so the one channel it must never
222+
// write to is the one the machine reads.
223+
expect(loaded.stdout).toBe('');
224+
}, 60_000);
225+
226+
it('the warning names every key, and says what to do instead', () => {
227+
const one = shadowedNamedExportWarning(['objects']).join('\n');
228+
expect(one).toContain('`objects`');
229+
expect(one).toContain('is a named export that was DROPPED');
230+
expect(one).toContain('move the value inside defineStack');
231+
232+
const many = shadowedNamedExportWarning(['objects', 'functions']).join('\n');
233+
expect(many).toContain('`objects`, `functions`');
234+
expect(many).toContain('are named exports that were DROPPED');
235+
});
236+
});

0 commit comments

Comments
 (0)