Skip to content

Commit

Permalink
fix(typescript): Handle null query parameters (#5622)
Browse files Browse the repository at this point in the history
  • Loading branch information
amckinney authored Jan 15, 2025
1 parent ee431ae commit 7b37211
Show file tree
Hide file tree
Showing 42 changed files with 868 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,25 @@ export namespace ConvertTypeReferenceParams {
return isInlinePropertyParams(params) || isInlineAliasParams(params);
}

export interface DefaultParams extends WithTypeReference {
export interface DefaultParams extends WithTypeReference, WithNullable {
type?: undefined;
}

/**
* Metadata for converting inline types
*/
export interface InlinePropertyTypeParams extends WithGenericIn, WithTypeReference {
export interface InlinePropertyTypeParams extends WithGenericIn, WithTypeReference, WithNullable {
type: "inlinePropertyParams";
parentTypeName: string;
propertyName: string;
}

export interface InlineAliasTypeParams extends WithGenericIn, WithTypeReference {
export interface InlineAliasTypeParams extends WithGenericIn, WithTypeReference, WithNullable {
type: "inlineAliasParams";
aliasTypeName: string;
}

export interface ForInlineUnionTypeParams extends WithTypeReference {
export interface ForInlineUnionTypeParams extends WithTypeReference, WithNullable {
type: "forInlineUnionParams";
}

Expand All @@ -77,6 +77,10 @@ export namespace ConvertTypeReferenceParams {
typeReference: TypeReference;
}

export interface WithNullable {
nullable?: boolean;
}

export const GenericIn = {
List: "list",
Map: "map",
Expand Down Expand Up @@ -117,7 +121,7 @@ export abstract class AbstractTypeReferenceConverter<T> {
public convert(params: ConvertTypeReferenceParams): T {
return TypeReference._visit<T>(params.typeReference, {
named: (type) => this.named(type, params),
primitive: (type) => this.primitive(type),
primitive: (type) => this.primitive(type, params),
container: (type) => this.container(type, params),
unknown: () => (this.treatUnknownAsAny ? this.any() : this.unknown()),
_other: () => {
Expand All @@ -142,11 +146,11 @@ export abstract class AbstractTypeReferenceConverter<T> {

protected abstract named(typeName: DeclaredTypeName, params: ConvertTypeReferenceParams): T;
protected abstract string(): T;
protected abstract number(): T;
protected abstract long(): T;
protected abstract bigInteger(): T;
protected abstract boolean(): T;
protected abstract dateTime(): T;
protected abstract number(params: ConvertTypeReferenceParams): T;
protected abstract long(params: ConvertTypeReferenceParams): T;
protected abstract bigInteger(params: ConvertTypeReferenceParams): T;
protected abstract boolean(params: ConvertTypeReferenceParams): T;
protected abstract dateTime(params: ConvertTypeReferenceParams): T;
protected abstract list(itemType: TypeReference, params: ConvertTypeReferenceParams): T;
protected abstract set(itemType: TypeReference, params: ConvertTypeReferenceParams): T;
protected abstract optional(itemType: TypeReference, params: ConvertTypeReferenceParams): T;
Expand All @@ -155,21 +159,21 @@ export abstract class AbstractTypeReferenceConverter<T> {
protected abstract unknown(): T;
protected abstract any(): T;

protected primitive(primitive: PrimitiveType): T {
protected primitive(primitive: PrimitiveType, params: ConvertTypeReferenceParams): T {
return PrimitiveTypeV1._visit<T>(primitive.v1, {
boolean: this.boolean.bind(this),
double: this.number.bind(this),
integer: this.number.bind(this),
long: this.long.bind(this),
float: this.number.bind(this),
uint: this.number.bind(this),
uint64: this.number.bind(this),
boolean: () => this.boolean(params),
double: () => this.number(params),
integer: () => this.number(params),
long: () => this.long(params),
float: () => this.number(params),
uint: () => this.number(params),
uint64: () => this.number(params),
string: this.string.bind(this),
uuid: this.string.bind(this),
dateTime: this.dateTime.bind(this),
dateTime: () => this.dateTime(params),
date: this.string.bind(this),
base64: this.string.bind(this),
bigInteger: this.bigInteger.bind(this),
bigInteger: () => this.bigInteger(params),
_other: () => {
throw new Error("Unexpected primitive type: " + primitive.v1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export abstract class AbstractTypeReferenceToTypeNodeConverter extends AbstractT
const resolvedType = this.context.type.resolveTypeName(typeName);
const isOptional = ResolvedTypeReference._visit<boolean>(resolvedType, {
container: (container) => this.container(container, params).isOptional,
primitive: (primitive) => this.primitive(primitive).isOptional,
primitive: (primitive) => this.primitive(primitive, params).isOptional,
named: () => false,
unknown: () => this.unknown().isOptional,
_other: () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ export class TypeReferenceToStringExpressionConverter extends AbstractTypeRefere
optional: (optionalType) => this.optional(optionalType, params),
set: this.set.bind(this),
map: (mapType) => this.map(mapType, params),
literal: this.literal.bind(this),
literal: (literal) => this.literal(literal, params),
_other: () => {
throw new Error("Unknown ContainerType: " + containerType.type);
}
}),
primitive: this.primitive.bind(this),
primitive: (type) => this.primitive(type, params),
named: ({ shape }) => {
if (shape === ShapeType.Enum) {
return (reference) => reference;
Expand All @@ -146,56 +146,28 @@ export class TypeReferenceToStringExpressionConverter extends AbstractTypeRefere
return (reference) => reference;
}

protected override number(): (reference: ts.Expression) => ts.Expression {
return (reference) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, "toString"),
undefined,
undefined
);
protected override number(params: ConvertTypeReferenceParams): (reference: ts.Expression) => ts.Expression {
return this.nullSafeCall("toString", params);
}

protected long(): (reference: ts.Expression) => ts.Expression {
if (this.useBigInt) {
return (reference) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, "toString"),
undefined,
undefined
);
}
return this.number();
protected long(params: ConvertTypeReferenceParams): (reference: ts.Expression) => ts.Expression {
return this.nullSafeCall("toString", params);
}

protected bigInteger(): (reference: ts.Expression) => ts.Expression {
protected bigInteger(params: ConvertTypeReferenceParams): (reference: ts.Expression) => ts.Expression {
if (this.useBigInt) {
return (reference) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, "toString"),
undefined,
undefined
);
return this.nullSafeCall("toString", params);
}
return this.string();
}

protected override boolean(): (reference: ts.Expression) => ts.Expression {
return (reference) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, "toString"),
undefined,
undefined
);
protected override boolean(params: ConvertTypeReferenceParams): (reference: ts.Expression) => ts.Expression {
return this.nullSafeCall("toString", params);
}

protected override dateTime(): (reference: ts.Expression) => ts.Expression {
protected override dateTime(params: ConvertTypeReferenceParams): (reference: ts.Expression) => ts.Expression {
if (this.includeSerdeLayer) {
return (reference) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, "toISOString"),
undefined,
undefined
);
return this.nullSafeCall("toISOString", params);
}
return (reference) => reference;
}
Expand All @@ -204,7 +176,7 @@ export class TypeReferenceToStringExpressionConverter extends AbstractTypeRefere
itemType: TypeReference,
params: ConvertTypeReferenceParams
): (reference: ts.Expression) => ts.Expression {
return (reference) => this.convert({ ...params, typeReference: itemType })(reference);
return (reference) => this.convert({ ...params, typeReference: itemType, nullable: true })(reference);
}

protected override optional(
Expand All @@ -226,15 +198,13 @@ export class TypeReferenceToStringExpressionConverter extends AbstractTypeRefere
return this.jsonStringify.bind(this);
}

protected override literal(literal: Literal): (reference: ts.Expression) => ts.Expression {
protected override literal(
literal: Literal,
params: ConvertTypeReferenceParams
): (reference: ts.Expression) => ts.Expression {
return Literal._visit(literal, {
string: () => (reference: ts.Expression) => reference,
boolean: () => (reference: ts.Expression) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, "toString"),
undefined,
undefined
),
boolean: () => (reference: ts.Expression) => this.nullSafeCall("toString", params)(reference),
_other: () => {
throw new Error("Unknown literal: " + literal.type);
}
Expand Down Expand Up @@ -335,4 +305,28 @@ export class TypeReferenceToStringExpressionConverter extends AbstractTypeRefere
[]
);
}

private nullSafeCall(
methodName: string,
params: ConvertTypeReferenceParams
): (reference: ts.Expression) => ts.Expression {
if (params.nullable) {
return (reference) =>
ts.factory.createBinaryExpression(
ts.factory.createPropertyAccessChain(
reference,
ts.factory.createToken(ts.SyntaxKind.QuestionDotToken),
ts.factory.createIdentifier(`${methodName}()`)
),
ts.factory.createToken(ts.SyntaxKind.QuestionQuestionToken),
ts.factory.createNull()
);
}
return (reference) =>
ts.factory.createCallExpression(
ts.factory.createPropertyAccessExpression(reference, methodName),
undefined,
undefined
);
}
}
12 changes: 12 additions & 0 deletions generators/typescript/sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.47.1] - 2025-01-15

- Fix: Resolves an issue where nullable query parameters were not null-safe in their method invocations. The
generated code now appropriately guard against `null` values like so:

```typescript
const _queryParams: Record< ... >;
if (value !== undefined) {
_queryParams["value"] = value?.toString() ?? null;
}
```

## [0.47.0] - 2025-01-14

- Feature: Add support for `nullable` properties. Users can now specify explicit `null` values
Expand Down
2 changes: 1 addition & 1 deletion generators/typescript/sdk/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.47.0
0.47.1
8 changes: 8 additions & 0 deletions packages/cli/cli/versions.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
- changelogEntry:
- summary: |
Fixes an issue where optional, nullable properties resulted in a double optional in the
IRv55 -> IRv54 migration.
type: internal
irVersion: 55
version: 0.50.3

- changelogEntry:
- summary: |
The docs now includes alpha support for parsing openrpc specs. To leverage this feature,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,40 @@
}
}
}
},
{
"name": {
"name": {
"originalName": "extra",
"camelCase": {
"unsafeName": "extra",
"safeName": "extra"
},
"snakeCase": {
"unsafeName": "extra",
"safeName": "extra"
},
"screamingSnakeCase": {
"unsafeName": "EXTRA",
"safeName": "EXTRA"
},
"pascalCase": {
"unsafeName": "Extra",
"safeName": "Extra"
}
},
"wireValue": "extra"
},
"typeReference": {
"type": "optional",
"value": {
"type": "nullable",
"value": {
"type": "primitive",
"value": "BOOLEAN"
}
}
}
}
],
"headers": [],
Expand Down
Loading

0 comments on commit 7b37211

Please sign in to comment.