Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/emitter-framework"
---

Use `Record<string, T>` extends instead of `additionalProperties` property for models that are or extend record types
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useTsp } from "../../core/context/tsp-context.js";
import { reportDiagnostic } from "../../lib.js";
import { declarationRefkeys, efRefkey } from "../utils/refkey.js";
import { InterfaceMember } from "./interface-member.js";
import { RecordExpression } from "./record-expression.js";
import { TypeExpression } from "./type-expression.jsx";
export interface TypedInterfaceDeclarationProps extends Omit<ts.InterfaceDeclarationProps, "name"> {
type: Model | Interface;
Expand Down Expand Up @@ -90,21 +91,26 @@ function getExtendsType($: Typekit, type: Model | Interface): Children | undefin
if ($.array.is(type.baseModel)) {
extending.push(<TypeExpression type={type.baseModel} />);
} else if ($.record.is(type.baseModel)) {
// Here we are in the additional properties land.
// Instead of extending we need to create an envelope property
// do nothing here.
extending.push(<RecordExpression elementType={type.baseModel.indexer!.value} />);
} else {
extending.push(efRefkey(type.baseModel));
}
}

const indexType = $.model.getIndexType(type);
if (indexType) {
// When extending a record we need to override the element type to be unknown to avoid type errors
if ($.record.is(indexType)) {
// Here we are in the additional properties land.
// Instead of extending we need to create an envelope property
// do nothing here.
const elementType = indexType.indexer!.value;
// Only use extends Record<string, T> if all known properties are assignable to T.
// When properties have incompatible types (e.g., id: number with Record<string, string>),
// skip the extends clause to avoid TypeScript index signature conflicts.
const properties = $.model.getProperties(type);
const allCompatible = Array.from(properties.values()).every((prop) =>
$.type.isAssignableTo(prop.type, elementType),
);
if (allCompatible) {
extending.push(<RecordExpression elementType={elementType} />);
}
} else {
extending.push(<TypeExpression type={indexType} />);
}
Expand All @@ -126,19 +132,31 @@ function getExtendsType($: Typekit, type: Model | Interface): Children | undefin
*/
function InterfaceBody(props: TypedInterfaceDeclarationProps): Children {
const { $ } = useTsp();
const additionalPropertiesKey = "additionalProperties";
let typeMembers: RekeyableMap<string, ModelProperty | Operation> | undefined;
if ($.model.is(props.type)) {
typeMembers = $.model.getProperties(props.type);
const additionalProperties = $.model.getAdditionalPropertiesRecord(props.type);
if (additionalProperties) {
typeMembers.set(
"additionalProperties",
$.modelProperty.create({
name: "additionalProperties",
optional: true,
type: additionalProperties,
}),

// When the model has a record indexer (from ...Record<T> spreads) but the named
// properties are not assignable to the element type, TypeScript cannot represent
// this with `extends Record<string, T>` (index signature compatibility constraint).
// Instead, add an `additionalProperties` named property to represent the extra properties.
const indexType = $.model.getIndexType(props.type);
if (indexType && $.record.is(indexType)) {
const elementType = indexType.indexer!.value;
const allCompatible = Array.from(typeMembers.values()).every((prop) =>
$.modelProperty.is(prop) ? $.type.isAssignableTo(prop.type, elementType) : true,
);
if (!allCompatible) {
typeMembers.set(
additionalPropertiesKey,
$.modelProperty.create({
name: additionalPropertiesKey,
optional: true,
type: indexType,
}),
);
}
}
} else {
typeMembers = createRekeyableMap(props.type.operations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,8 @@ describe("Typescript Interface", () => {
</SourceFile>
</Output>,
).toRenderTo(`
export interface DifferentSpreadModelRecord {
export interface DifferentSpreadModelRecord extends Record<string, unknown> {
knownProp: string;
additionalProperties?: Record<string, unknown>;
}
`);
});
Expand Down Expand Up @@ -235,9 +234,7 @@ describe("Typescript Interface", () => {
</SourceFile>
</Output>,
).toRenderTo(`
export interface Foo {
additionalProperties?: Record<string, string>;
}`);
export interface Foo extends Record<string, string> {}`);
});

it("creates an interface of a model that spreads a Record", async () => {
Expand All @@ -261,9 +258,7 @@ describe("Typescript Interface", () => {
</SourceFile>
</Output>,
).toRenderTo(`
export interface Foo {
additionalProperties?: Record<string, string>;
}
export interface Foo extends Record<string, string> {}
`);
});

Expand Down Expand Up @@ -306,7 +301,6 @@ describe("Typescript Interface", () => {
}
export interface DifferentSpreadModelDerived extends DifferentSpreadModelRecord {
derivedProp: ModelForRecord;
additionalProperties?: Record<string, ModelForRecord>;
}`);
});

Expand All @@ -332,11 +326,65 @@ describe("Typescript Interface", () => {
</SourceFile>
</Output>,
).toRenderTo(`
export interface Widget {
export interface Widget extends Record<string, unknown> {
id: string;
weight: number;
color: "blue" | "red";
additionalProperties?: Record<string, unknown>;
}`);
});

it("adds additionalProperties when known properties are incompatible with the record element type", async () => {
const program = await getProgram(`
namespace DemoService;
model Widget {
id: int32;
...Record<string>;
}
`);

const [namespace] = program.resolveTypeReference("DemoService");
const models = Array.from((namespace as Namespace).models.values());

expect(
<Output program={program}>
<SourceFile path="test.ts">
{models.map((model) => (
<InterfaceDeclaration export type={model} />
))}
</SourceFile>
</Output>,
).toRenderTo(`
export interface Widget {
id: number;
additionalProperties?: Record<string, string>;
}`);
});

it("adds additionalProperties for multiple incompatible record spreads as a union type", async () => {
const program = await getProgram(`
namespace DemoService;
model Foo {
id: int32;
...Record<string>;
...Record<float32>;
}
`);

const [namespace] = program.resolveTypeReference("DemoService");
const models = Array.from((namespace as Namespace).models.values());

expect(
<Output program={program}>
<SourceFile path="test.ts">
{models.map((model) => (
<InterfaceDeclaration export type={model} />
))}
</SourceFile>
</Output>,
).toRenderTo(`
export interface Foo {
id: number;
additionalProperties?: Record<string, string | number>;
}`);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,36 @@ export interface JsonAdditionalPropertiesTransformProps {
target: "transport" | "application";
}

/**
* Determines whether a model uses the `additionalProperties` wrapper property
* (instead of flat spreading). This is true when the model has a record index type
* from spreads but the named properties are not all assignable to the element type,
* or when an ancestor model uses the wrapper approach.
*/
function usesWrappedAdditionalProperties($: ReturnType<typeof useTsp>["$"], type: Model): boolean {
const indexType = $.model.getIndexType(type);
if (indexType && $.record.is(indexType)) {
const elementType = indexType.indexer!.value;
const properties = $.model.getProperties(type);
const allCompatible = Array.from(properties.values()).every((prop) =>
$.modelProperty.is(prop) ? $.type.isAssignableTo(prop.type, elementType) : true,
);
return !allCompatible;
}

// Direct base is a record (extends Record<T>) → always flat
if (type.baseModel && $.record.is(type.baseModel)) {
return false;
}

// Recurse into base model to check if the wrapper comes from inheritance
if (type.baseModel) {
return usesWrappedAdditionalProperties($, type.baseModel);
}

return false;
}

export function JsonAdditionalPropertiesTransform(props: JsonAdditionalPropertiesTransformProps) {
const { $ } = useTsp();
const additionalProperties = $.model.getAdditionalPropertiesRecord(props.type);
Expand All @@ -18,36 +48,47 @@ export function JsonAdditionalPropertiesTransform(props: JsonAdditionalPropertie
return null;
}

if (props.target === "application") {
const properties = $.model.getProperties(props.type, { includeExtended: true });
const destructuredProperties = mapJoin(
() => properties,
(name) => name,
{
joiner: ",",
ender: ",",
},
);
const properties = $.model.getProperties(props.type, { includeExtended: true });
const destructuredProperties = mapJoin(
() => properties,
(name) => name,
{
joiner: ",",
ender: ",",
},
);

if (usesWrappedAdditionalProperties($, props.type)) {
// Incompatible case: use additionalProperties wrapper property
if (props.target === "application") {
const inlineDestructure = code`
${getJsonRecordTransformRefkey(additionalProperties, props.target)}(
(({ ${destructuredProperties} ...rest }) => rest)(${props.itemRef})
),
`;

return (
<>
<ts.ObjectProperty name="additionalProperties">{inlineDestructure}</ts.ObjectProperty>
</>
);
}

// Inline destructuring that extracts the properties and passes the rest to jsonRecordUnknownToApplicationTransform_2
const inlineDestructure = code`
${getJsonRecordTransformRefkey(additionalProperties, props.target)}(
(({ ${destructuredProperties} ...rest }) => rest)(${props.itemRef})
),
`;
const itemRef = code`${props.itemRef}.additionalProperties`;

return (
<>
<ts.ObjectProperty name="additionalProperties">{inlineDestructure}</ts.ObjectProperty>
...({getJsonRecordTransformRefkey(additionalProperties, props.target)}({itemRef}) ),
</>
);
}

const itemRef = code`${props.itemRef}.additionalProperties`;
// Compatible case: spread additional properties directly onto the object
const restExpr = code`(({ ${destructuredProperties} ...rest }) => rest)(${props.itemRef})`;

return (
<>
...({getJsonRecordTransformRefkey(additionalProperties, props.target)}({itemRef}) ),
...({getJsonRecordTransformRefkey(additionalProperties, props.target)}({restExpr}) ),
</>
);
}
Loading