Skip to content

Commit 88fa2ad

Browse files
mathpirateclaude
mathpirate
and
claude
authored
Fix/record mapped type schemas (#1880)
* fix: resolve mapped type properties correctly Mapped types (like Record<K, V> and inline mapped types) have synthetic properties without explicit declarations. Updated safeGetPropertyType() to use checker.getPropertyOfType() + getTypeOfSymbol() to resolve these property types correctly, preventing them from falling back to 'any' (which generates permissive 'true' schemas). Added fixture test for inline mapped types to prevent regression. * switch to filtering out types from being named based on whether they're parametrized generics rather than whether they're library functions * fmt * refactor: cleanup and add edge case tests for Record types Cleanup: - Reverted aliasSymbol extraction to keep diff minimal - Reverted targetSymbol scope to original tighter scope - Both changes maintain exact same behavior, just cleaner New edge case tests: - Record<string, T> with additionalProperties - Generic types with default parameters - Conditional types resolving to Records * fix: handle generic type instantiation and optional properties in safeGetPropertyType - Compare parent vs declaration types to detect generic instantiation (e.g., Box<number>) - Strip | undefined from optional properties using type flags instead of string matching - Handle Partial<T> and other mapped types correctly - Fix recursion tests by preferring declaration type for optional properties * correct test expectation * fmt --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 715e7df commit 88fa2ad

File tree

6 files changed

+624
-6
lines changed

6 files changed

+624
-6
lines changed

‎packages/schema-generator/src/type-utils.ts‎

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,114 @@ export function safeGetPropertyType(
7272
checker: ts.TypeChecker,
7373
fallbackNode?: ts.TypeNode,
7474
): ts.Type {
75-
// Prefer declared type node when available
7675
const decl = prop.valueDeclaration;
76+
const isOptional = (prop.flags & ts.SymbolFlags.Optional) !== 0;
77+
78+
// Get type from the resolved parent type (for generic instantiation and mapped types)
79+
// We'll use this to validate/override the declaration-based type if needed
80+
let typeFromParent: ts.Type | undefined;
81+
try {
82+
const propName = prop.getName();
83+
const propSymbol = checker.getPropertyOfType(parentType, propName);
84+
if (propSymbol) {
85+
typeFromParent = checker.getTypeOfSymbol(propSymbol);
86+
const typeStr = typeFromParent
87+
? checker.typeToString(typeFromParent)
88+
: undefined;
89+
// If we got 'any', treat it as if we didn't get a type
90+
if (typeStr === "any") {
91+
typeFromParent = undefined;
92+
}
93+
}
94+
} catch {
95+
// Type resolution can fail for some edge cases
96+
}
97+
98+
// Try to get type from declaration
99+
let typeFromDecl: ts.Type | undefined;
77100
if (decl && ts.isPropertySignature(decl) && decl.type) {
78-
const typeFromNode = safeGetTypeFromTypeNode(
101+
typeFromDecl = safeGetTypeFromTypeNode(
79102
checker,
80103
decl.type,
81104
"property signature",
82105
);
83-
if (typeFromNode) return typeFromNode;
84106
}
85107

108+
// If we have both, and they differ, prefer parent (handles generic instantiation)
109+
// Example: Box<number> where property is declared as `value: T` but should resolve to `number`
110+
if (typeFromParent && typeFromDecl) {
111+
const parentStr = checker.typeToString(typeFromParent);
112+
const declStr = checker.typeToString(typeFromDecl);
113+
114+
if (parentStr !== declStr) {
115+
// For optional properties, the parent type may include "| undefined" which we don't want
116+
// The optionality is tracked separately in the schema via the required array
117+
// Check if parent is a union that contains undefined, and if removing it gives us the decl type
118+
if (isOptional && typeFromParent.isUnion()) {
119+
const parentUnion = typeFromParent as ts.UnionType;
120+
const hasUndefined = parentUnion.types.some((t) =>
121+
!!(t.flags & ts.TypeFlags.Undefined)
122+
);
123+
124+
if (hasUndefined) {
125+
const nonUndefinedTypes = parentUnion.types.filter(
126+
(t) => !(t.flags & ts.TypeFlags.Undefined),
127+
);
128+
129+
// Compare the non-undefined part with the declaration type
130+
// Handle both single types and remaining unions
131+
if (nonUndefinedTypes.length === 1 && nonUndefinedTypes[0]) {
132+
const withoutUndefined = checker.typeToString(nonUndefinedTypes[0]);
133+
if (withoutUndefined === declStr) {
134+
// Parent only differs by the added | undefined, use decl
135+
return typeFromDecl;
136+
}
137+
} else if (nonUndefinedTypes.length > 1) {
138+
// Multiple non-undefined types - check if decl is also a union with the same types
139+
// For now, just check string equality which should work for most cases
140+
const withoutUndefinedStr = nonUndefinedTypes.map((t) =>
141+
checker.typeToString(t)
142+
).join(" | ");
143+
if (
144+
withoutUndefinedStr === declStr || declStr === withoutUndefinedStr
145+
) {
146+
return typeFromDecl;
147+
}
148+
}
149+
}
150+
}
151+
152+
// Types differ - parent has the instantiated/resolved version (e.g., T -> number)
153+
return typeFromParent;
154+
}
155+
}
156+
157+
// If only parent type available (e.g., mapped types with no declaration), use it
158+
if (typeFromParent && !typeFromDecl) {
159+
// For optional properties from mapped types (like Partial<T>), the parent type
160+
// includes "| undefined", but we want just the base type since optionality
161+
// is tracked separately in the required array
162+
if (isOptional && typeFromParent.isUnion()) {
163+
const unionType = typeFromParent as ts.UnionType;
164+
const nonUndefinedTypes = unionType.types.filter(
165+
(t) => !(t.flags & ts.TypeFlags.Undefined),
166+
);
167+
if (nonUndefinedTypes.length === 1 && nonUndefinedTypes[0]) {
168+
return nonUndefinedTypes[0];
169+
}
170+
// If multiple non-undefined types, just use parent as-is
171+
// The schema generator will handle the union properly
172+
}
173+
174+
return typeFromParent;
175+
}
176+
177+
// Otherwise use declaration type
178+
if (typeFromDecl) {
179+
return typeFromDecl;
180+
}
181+
182+
// Fallback to provided node
86183
if (fallbackNode) {
87184
const typeFromFallback = safeGetTypeFromTypeNode(
88185
checker,
@@ -92,7 +189,7 @@ export function safeGetPropertyType(
92189
if (typeFromFallback) return typeFromFallback;
93190
}
94191

95-
// Last resort: use symbol location
192+
// Try symbol location as last resort before giving up
96193
if (decl) {
97194
const typeFromSymbol = safeGetTypeOfSymbolAtLocation(
98195
checker,
@@ -103,7 +200,7 @@ export function safeGetPropertyType(
103200
if (typeFromSymbol) return typeFromSymbol;
104201
}
105202

106-
// If all else fails, return any
203+
// Absolute last resort - return 'any' type
107204
return checker.getAnyType();
108205
}
109206

@@ -203,7 +300,6 @@ export function getNativeTypeSchema(
203300

204301
return resolve(type);
205302
}
206-
207303
/**
208304
* Return a public/stable named key for a type if and only if it has a useful
209305
* symbol name. Filters out anonymous ("__type") and wrapper/container names
@@ -280,6 +376,19 @@ export function getNamedTypeKey(
280376
return undefined;
281377
}
282378
if (name && NATIVE_TYPE_NAMES.has(name)) return undefined;
379+
380+
// Don't hoist generic type instantiations (Record<K,V>, Partial<T>, Box<T>, etc.)
381+
// These have aliasTypeArguments, meaning they're a generic type applied to specific type arguments
382+
// The name "Record" or "Box" is meaningless without the type parameters - what matters is the
383+
// resolved/instantiated type structure
384+
const typeWithAlias = type as TypeWithInternals;
385+
if (
386+
typeWithAlias.aliasTypeArguments &&
387+
typeWithAlias.aliasTypeArguments.length > 0
388+
) {
389+
return undefined;
390+
}
391+
283392
return name;
284393
}
285394

‎packages/schema-generator/test/fixtures/schema/inline-mapped-type.expected.json‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
{
2+
"type": "object",
3+
"properties": {
4+
"stringConfig": {
5+
"type": "object",
6+
"properties": {
7+
"theme": {
8+
"type": "string"
9+
},
10+
"language": {
11+
"type": "string"
12+
},
13+
"timezone": {
14+
"type": "string"
15+
}
16+
},
17+
"required": [
18+
"theme",
19+
"language",
20+
"timezone"
21+
]
22+
},
23+
"numberLimits": {
24+
"type": "object",
25+
"properties": {
26+
"cpu": {
27+
"type": "number"
28+
},
29+
"memory": {
30+
"type": "number"
31+
},
32+
"disk": {
33+
"type": "number"
34+
}
35+
},
36+
"required": [
37+
"cpu",
38+
"memory",
39+
"disk"
40+
]
41+
},
42+
"booleanFeatures": {
43+
"type": "object",
44+
"properties": {
45+
"auth": {
46+
"type": "boolean"
47+
},
48+
"api": {
49+
"type": "boolean"
50+
},
51+
"ui": {
52+
"type": "boolean"
53+
}
54+
},
55+
"required": [
56+
"auth",
57+
"api",
58+
"ui"
59+
]
60+
},
61+
"objectHandlers": {
62+
"type": "object",
63+
"properties": {
64+
"create": {
65+
"type": "object",
66+
"properties": {
67+
"enabled": {
68+
"type": "boolean"
69+
}
70+
},
71+
"required": [
72+
"enabled"
73+
]
74+
},
75+
"update": {
76+
"type": "object",
77+
"properties": {
78+
"enabled": {
79+
"type": "boolean"
80+
}
81+
},
82+
"required": [
83+
"enabled"
84+
]
85+
},
86+
"delete": {
87+
"type": "object",
88+
"properties": {
89+
"enabled": {
90+
"type": "boolean"
91+
}
92+
},
93+
"required": [
94+
"enabled"
95+
]
96+
}
97+
},
98+
"required": [
99+
"create",
100+
"update",
101+
"delete"
102+
]
103+
}
104+
},
105+
"required": [
106+
"stringConfig",
107+
"numberLimits",
108+
"booleanFeatures",
109+
"objectHandlers"
110+
]
111+
}

‎packages/schema-generator/test/fixtures/schema/inline-mapped-type.input.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
interface SchemaRoot {
2+
stringConfig: {
3+
[P in "theme" | "language" | "timezone"]: string;
4+
};
5+
numberLimits: {
6+
[K in "cpu" | "memory" | "disk"]: number;
7+
};
8+
booleanFeatures: {
9+
[F in "auth" | "api" | "ui"]: boolean;
10+
};
11+
objectHandlers: {
12+
[H in "create" | "update" | "delete"]: { enabled: boolean };
13+
};
14+
}

‎packages/schema-generator/test/fixtures/schema/record-union-keys.expected.json‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
{
2+
"type": "object",
3+
"properties": {
4+
"settings": {
5+
"type": "object",
6+
"properties": {
7+
"theme": {
8+
"type": "string"
9+
},
10+
"language": {
11+
"type": "string"
12+
},
13+
"timezone": {
14+
"type": "string"
15+
}
16+
},
17+
"required": [
18+
"theme",
19+
"language",
20+
"timezone"
21+
]
22+
},
23+
"limits": {
24+
"type": "object",
25+
"properties": {
26+
"cpu": {
27+
"type": "number"
28+
},
29+
"memory": {
30+
"type": "number"
31+
},
32+
"disk": {
33+
"type": "number"
34+
}
35+
},
36+
"required": [
37+
"cpu",
38+
"memory",
39+
"disk"
40+
]
41+
},
42+
"features": {
43+
"type": "object",
44+
"properties": {
45+
"auth": {
46+
"type": "boolean"
47+
},
48+
"api": {
49+
"type": "boolean"
50+
},
51+
"ui": {
52+
"type": "boolean"
53+
}
54+
},
55+
"required": [
56+
"auth",
57+
"api",
58+
"ui"
59+
]
60+
}
61+
},
62+
"required": [
63+
"settings",
64+
"limits",
65+
"features"
66+
]
67+
}

‎packages/schema-generator/test/fixtures/schema/record-union-keys.input.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
interface SchemaRoot {
2+
settings: Record<"theme" | "language" | "timezone", string>;
3+
limits: Record<"cpu" | "memory" | "disk", number>;
4+
features: Record<"auth" | "api" | "ui", boolean>;
5+
}

0 commit comments

Comments
 (0)