Skip to content

Commit c46aa18

Browse files
dahliaclaude
andcommitted
Fix annotation injection corrupting undefined state
injectAnnotationsIntoParser() unconditionally spread parser.initialState into a new object with the annotation key. When initialState was undefined (as in withDefault() and optional()), this turned it into { [annotationKey]: annotations }, breaking the typeof state === "undefined" check that parseOptionalStyleSync() relies on to distinguish the uninitialized state from a wrapped [TState] tuple. The fix skips annotation injection when initialState is null or undefined. The same guard is applied to all annotation injection sites in parseSync(), parseAsync(), suggestSync(), suggestAsync(), and getDocPage(). Fixes #131 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 53b5870 commit c46aa18

File tree

4 files changed

+104
-31
lines changed

4 files changed

+104
-31
lines changed

CHANGES.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,19 @@ To be released.
1010

1111
### @optique/core
1212

13+
- Fixed `runWith()` (and by extension `runWithConfig()`) crashing with
14+
`TypeError: Cannot read properties of undefined` when the top-level parser
15+
has an `undefined` initial state, such as `withDefault(object({...}))`.
16+
The root cause was that `injectAnnotationsIntoParser()` unconditionally
17+
spread `parser.initialState` into a new object with the annotation key,
18+
turning `undefined` into `{ [annotationKey]: annotations }`. This corrupted
19+
the state for parsers like `withDefault()` and `optional()` that rely on
20+
`typeof state === "undefined"` to distinguish the uninitialized state from
21+
a wrapped inner state. The fix skips annotation injection when the initial
22+
state is `null` or `undefined`. The same guard was applied to annotation
23+
injection in `parseSync()`, `parseAsync()`, `suggestSync()`,
24+
`suggestAsync()`, and `getDocPage()`. [[#131]]
25+
1326
- Fixed `formatDocPage()` to respect `maxWidth` when the rendered option
1427
term is wider than `termWidth` (default: 26). Previously, the description
1528
column width was calculated assuming the term occupied exactly `termWidth`
@@ -27,6 +40,7 @@ To be released.
2740
was appended after `formatMessage()` had already filled the description
2841
column to capacity, producing lines that were one character too wide.
2942

43+
[#131]: https://github.com/dahlia/optique/issues/131
3044
[#132]: https://github.com/dahlia/optique/issues/132
3145

3246

packages/config/src/run.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
44
import { join } from "node:path";
55
import { z } from "zod";
66
import { object } from "@optique/core/constructs";
7-
import { option } from "@optique/core/primitives";
7+
import { flag, option } from "@optique/core/primitives";
88
import { integer, string } from "@optique/core/valueparser";
99
import { withDefault } from "@optique/core/modifiers";
1010
import { bindConfig, createConfigContext } from "./index.ts";
@@ -639,4 +639,49 @@ describe("runWithConfig", { concurrency: false }, () => {
639639
assert.ok(helpOutput.includes("Usage: my-custom-app"));
640640
});
641641
});
642+
643+
test("withDefault(object(...)) returns default on empty args", async () => {
644+
// Regression test for https://github.com/dahlia/optique/issues/131
645+
// runWithConfig() crashed with TypeError when withDefault(object(...))
646+
// took the default path (no tokens consumed).
647+
const schema = z.object({});
648+
const context = createConfigContext({ schema });
649+
650+
const parser = withDefault(
651+
object({
652+
enabled: flag("--enabled"),
653+
dependent: option("--dependent", string()),
654+
}),
655+
{ enabled: false as const } as const,
656+
);
657+
658+
const result = await runWithConfig(parser, context, {
659+
load: () => ({}),
660+
args: [],
661+
});
662+
663+
assert.deepEqual(result, { enabled: false });
664+
});
665+
666+
test("withDefault(object(...)) returns parsed value when args given", async () => {
667+
// Companion test for https://github.com/dahlia/optique/issues/131
668+
// Verify that when tokens are consumed, the parser works correctly.
669+
const schema = z.object({});
670+
const context = createConfigContext({ schema });
671+
672+
const parser = withDefault(
673+
object({
674+
enabled: flag("--enabled"),
675+
dependent: option("--dependent", string()),
676+
}),
677+
{ enabled: false as const } as const,
678+
);
679+
680+
const result = await runWithConfig(parser, context, {
681+
load: () => ({}),
682+
args: ["--enabled", "--dependent", "foo"],
683+
});
684+
685+
assert.deepEqual(result, { enabled: true, dependent: "foo" });
686+
});
642687
});

packages/core/src/facade.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2650,6 +2650,14 @@ function injectAnnotationsIntoParser<
26502650
parser: Parser<M, TValue, TState>,
26512651
annotations: Annotations,
26522652
): Parser<M, TValue, TState> {
2653+
// Skip annotation injection for null/undefined initial states (e.g.,
2654+
// undefined for withDefault/optional) to preserve their semantic meaning.
2655+
// Corrupting undefined to { [annotationKey]: ... } breaks parsers that
2656+
// rely on typeof state === "undefined" checks.
2657+
if (parser.initialState == null) {
2658+
return parser;
2659+
}
2660+
26532661
// Create a new initial state with annotations
26542662
const newInitialState = {
26552663
...parser.initialState,

packages/core/src/parser.ts

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -383,12 +383,13 @@ export function parseSync<T>(
383383
): Result<T> {
384384
let initialState = parser.initialState;
385385

386-
// Inject annotations into state if provided
387-
if (options?.annotations) {
386+
// Inject annotations into state if provided, but skip for null/undefined
387+
// initial states (e.g., undefined for withDefault/optional) to preserve
388+
// their semantic meaning. Corrupting undefined to { [annotationKey]: ... }
389+
// breaks parsers that rely on typeof state === "undefined" checks.
390+
if (options?.annotations && initialState != null) {
388391
initialState = {
389-
...(typeof initialState === "object" && initialState !== null
390-
? initialState
391-
: {}),
392+
...(typeof initialState === "object" ? initialState : {}),
392393
[annotationKey]: options.annotations,
393394
} as unknown;
394395
}
@@ -452,12 +453,13 @@ export async function parseAsync<T>(
452453
): Promise<Result<T>> {
453454
let initialState = parser.initialState;
454455

455-
// Inject annotations into state if provided
456-
if (options?.annotations) {
456+
// Inject annotations into state if provided, but skip for null/undefined
457+
// initial states (e.g., undefined for withDefault/optional) to preserve
458+
// their semantic meaning. Corrupting undefined to { [annotationKey]: ... }
459+
// breaks parsers that rely on typeof state === "undefined" checks.
460+
if (options?.annotations && initialState != null) {
457461
initialState = {
458-
...(typeof initialState === "object" && initialState !== null
459-
? initialState
460-
: {}),
462+
...(typeof initialState === "object" ? initialState : {}),
461463
[annotationKey]: options.annotations,
462464
} as unknown;
463465
}
@@ -578,12 +580,13 @@ export function suggestSync<T>(
578580

579581
let initialState = parser.initialState;
580582

581-
// Inject annotations into state if provided
582-
if (options?.annotations) {
583+
// Inject annotations into state if provided, but skip for null/undefined
584+
// initial states (e.g., undefined for withDefault/optional) to preserve
585+
// their semantic meaning. Corrupting undefined to { [annotationKey]: ... }
586+
// breaks parsers that rely on typeof state === "undefined" checks.
587+
if (options?.annotations && initialState != null) {
583588
initialState = {
584-
...(typeof initialState === "object" && initialState !== null
585-
? initialState
586-
: {}),
589+
...(typeof initialState === "object" ? initialState : {}),
587590
[annotationKey]: options.annotations,
588591
} as unknown;
589592
}
@@ -650,12 +653,13 @@ export async function suggestAsync<T>(
650653

651654
let initialState = parser.initialState;
652655

653-
// Inject annotations into state if provided
654-
if (options?.annotations) {
656+
// Inject annotations into state if provided, but skip for null/undefined
657+
// initial states (e.g., undefined for withDefault/optional) to preserve
658+
// their semantic meaning. Corrupting undefined to { [annotationKey]: ... }
659+
// breaks parsers that rely on typeof state === "undefined" checks.
660+
if (options?.annotations && initialState != null) {
655661
initialState = {
656-
...(typeof initialState === "object" && initialState !== null
657-
? initialState
658-
: {}),
662+
...(typeof initialState === "object" ? initialState : {}),
659663
[annotationKey]: options.annotations,
660664
} as unknown;
661665
}
@@ -916,12 +920,13 @@ function getDocPageSyncImpl(
916920
): DocPage | undefined {
917921
let initialState = parser.initialState;
918922

919-
// Inject annotations into state if provided
920-
if (options?.annotations) {
923+
// Inject annotations into state if provided, but skip for null/undefined
924+
// initial states (e.g., undefined for withDefault/optional) to preserve
925+
// their semantic meaning. Corrupting undefined to { [annotationKey]: ... }
926+
// breaks parsers that rely on typeof state === "undefined" checks.
927+
if (options?.annotations && initialState != null) {
921928
initialState = {
922-
...(typeof initialState === "object" && initialState !== null
923-
? initialState
924-
: {}),
929+
...(typeof initialState === "object" ? initialState : {}),
925930
[annotationKey]: options.annotations,
926931
} as unknown;
927932
}
@@ -950,12 +955,13 @@ async function getDocPageAsyncImpl(
950955
): Promise<DocPage | undefined> {
951956
let initialState = parser.initialState;
952957

953-
// Inject annotations into state if provided
954-
if (options?.annotations) {
958+
// Inject annotations into state if provided, but skip for null/undefined
959+
// initial states (e.g., undefined for withDefault/optional) to preserve
960+
// their semantic meaning. Corrupting undefined to { [annotationKey]: ... }
961+
// breaks parsers that rely on typeof state === "undefined" checks.
962+
if (options?.annotations && initialState != null) {
955963
initialState = {
956-
...(typeof initialState === "object" && initialState !== null
957-
? initialState
958-
: {}),
964+
...(typeof initialState === "object" ? initialState : {}),
959965
[annotationKey]: options.annotations,
960966
} as unknown;
961967
}

0 commit comments

Comments
 (0)