Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark schema objects with default values as non-nullable #613

Merged
merged 1 commit into from
May 27, 2021
Merged
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
12 changes: 9 additions & 3 deletions src/transform/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@ interface TransformSchemaObjOptions extends GlobalContext {
required: Set<string>;
}

function hasDefaultValue(node: any): boolean {
if (node.hasOwnProperty("default")) return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change here: check if a schema object has a default value. But like the description states, we can’t currently look this up for $ref (which might be a remote schema)

// if (node.hasOwnProperty("$ref")) return true; // TODO: resolve remote $refs?
return false;
}

/** Take object keys and convert to TypeScript interface */
export function transformSchemaObjMap(obj: Record<string, any>, options: TransformSchemaObjOptions): string {
const readonly = tsReadonly(options.immutableTypes);

let output = "";

for (const k of Object.keys(obj)) {
Expand All @@ -27,7 +31,9 @@ export function transformSchemaObjMap(obj: Record<string, any>, options: Transfo
if (v.description) output += comment(v.description);

// 2. name (with “?” if optional property)
output += `${readonly}"${k}"${options.required.has(k) ? "" : "?"}: `;
const readonly = tsReadonly(options.immutableTypes);
const required = options.required.has(k) || hasDefaultValue(v.schema || v) ? "" : "?";
output += `${readonly}"${k}"${required}: `;

// 3. transform
output += transformSchemaObj(v.schema || v, options);
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export interface SchemaObject {
items?: ReferenceObject | SchemaObject;
allOf?: SchemaObject;
properties?: Record<string, ReferenceObject | SchemaObject>;
default?: any;
additionalProperties?: boolean | ReferenceObject | SchemaObject;
nullable?: boolean; // V3 ONLY
oneOf?: (ReferenceObject | SchemaObject)[]; // V3 ONLY
Expand Down
2 changes: 1 addition & 1 deletion tests/bin/expected/prettier-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export interface components {
shipDate?: string
/** Order Status */
status?: 'placed' | 'approved' | 'delivered'
complete?: boolean
complete: boolean
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated snapshots show it’s improved a lot of schemas

}
Category: {
id?: number
Expand Down
2 changes: 1 addition & 1 deletion tests/bin/expected/prettier-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export interface components {
shipDate?: string
/** Order Status */
status?: 'placed' | 'approved' | 'delivered'
complete?: boolean
complete: boolean
}
Category: {
id?: number
Expand Down
2 changes: 1 addition & 1 deletion tests/bin/expected/stdout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export interface components {
shipDate?: string;
/** Order Status */
status?: "placed" | "approved" | "delivered";
complete?: boolean;
complete: boolean;
};
Category: {
id?: number;
Expand Down
38 changes: 19 additions & 19 deletions tests/v2/expected/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,11 +618,11 @@ export interface definitions {
base_url?: string;
sso_url?: string;
version?: "v1";
features?: {
features: {
access_code?: boolean;
sso?: boolean;
plan_change?: boolean;
credential?: "none" | "single" | "multiple" | "unknown";
credential: "none" | "single" | "multiple" | "unknown";
};
};
/** An array of platform ids to restrict this product for. */
Expand Down Expand Up @@ -662,24 +662,24 @@ export interface definitions {
name: definitions["Name"];
type: "boolean" | "string" | "number";
/** This sets whether or not the feature can be customized by a consumer. */
customizable?: boolean;
customizable: boolean;
/**
* This sets whether or not the feature can be upgraded by the consumer after the
* resource has provisioned. Upgrading means setting a higher value or selecting a
* higher element in the list.
*/
upgradable?: boolean;
upgradable: boolean;
/**
* This sets whether or not the feature can be downgraded by the consumer after the
* resource has provisioned. Downgrading means setting a lower value or selecting a
* lower element in the list.
*/
downgradable?: boolean;
downgradable: boolean;
/**
* Sets if this feature’s value is trackable from the provider,
* this only really affects numeric constraints.
*/
measurable?: boolean;
measurable: boolean;
values?: definitions["FeatureValuesList"];
};
/**
Expand All @@ -699,7 +699,7 @@ export interface definitions {
* is selected or is default for the plan.
* Cost is deprecated in favor of the `price.cost` field.
*/
cost?: number;
cost: number;
/**
* Price describes the cost of a feature. It should be preferred over
* the `cost` property.
Expand All @@ -710,13 +710,13 @@ export interface definitions {
* when this value is selected or is default for the plan.
* Number features should use the cost range instead.
*/
cost?: number;
cost: number;
/**
* When a feature is used to multiply the cost of the plan or of
* another feature, multiply factor is used for calculation.
* A feature cannot have both a cost and a multiply factor.
*/
multiply_factor?: number;
multiply_factor: number;
/** Price describes how the feature cost should be calculated. */
formula?: definitions["PriceFormula"];
/** Description explains how a feature is calculated to the user. */
Expand All @@ -737,9 +737,9 @@ export interface definitions {
* means this numeric details has no scale, and will not be or customizable.
* Some plans may not have a measureable or customizable feature.
*/
increment?: number;
increment: number;
/** Minimum value that can be set by a user if customizable */
min?: number;
min: number;
/** Maximum value that can be set by a user if customizable */
max?: number;
/** Applied to the end of the number for display, for example the ‘GB’ in ‘20 GB’. */
Expand All @@ -758,7 +758,7 @@ export interface definitions {
* An integer in 10,000,000ths of cents, will be multiplied by the
* numeric value set in the feature to determine the cost.
*/
cost_multiple?: number;
cost_multiple: number;
};
FeatureValue: {
feature: definitions["Label"];
Expand All @@ -784,40 +784,40 @@ export interface definitions {
* When true, everyone can see the product when requested. When false it will
* not be visible to anyone except those on the provider team.
*/
public?: boolean;
public: boolean;
/**
* When true, the product will be displayed in product listings alongside
* other products. When false the product will be excluded from listings,
* but can still be provisioned directly if it's label is known.
* Any pages that display information about the product when not listed,
* should indicate to webcrawlers that the content should not be indexed.
*/
listed?: boolean;
listed: boolean;
/**
* Object to hold various flags for marketing purposes only. These are values
* that need to be stored, but should not affect decision making in code. If
* we find ourselves in a position where we think they should, we should
* consider refactoring our listing definition.
*/
marketing?: {
marketing: {
/**
* Indicates whether or not the product is in `Beta` and should be
* advertised as such. This does not have any impact on who can access the
* product, it is just used to inform consumers through our clients.
*/
beta?: boolean;
beta: boolean;
/**
* Indicates whether or not the product is in `New` and should be
* advertised as such. This does not have any impact on who can access the
* product, it is just used to inform consumers through our clients.
*/
new?: boolean;
new: boolean;
/**
* Indicates whether or not the product is in `New` and should be
* advertised as such. This does not have any impact on who can access the
* product, it is just used to inform consumers through our clients.
*/
featured?: boolean;
featured: boolean;
};
};
/**
Expand Down Expand Up @@ -858,7 +858,7 @@ export interface definitions {
* * `multiple`: Multiple credentials are supported at the same time.
* * `unknown`: The credential type is unknown.
*/
credential?: "none" | "single" | "multiple" | "unknown";
credential: "none" | "single" | "multiple" | "unknown";
};
ProductBody: {
provider_id: definitions["ID"];
Expand Down
38 changes: 19 additions & 19 deletions tests/v2/expected/manifold.immutable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,11 +618,11 @@ export interface definitions {
readonly base_url?: string;
readonly sso_url?: string;
readonly version?: "v1";
readonly features?: {
readonly features: {
readonly access_code?: boolean;
readonly sso?: boolean;
readonly plan_change?: boolean;
readonly credential?: "none" | "single" | "multiple" | "unknown";
readonly credential: "none" | "single" | "multiple" | "unknown";
};
};
/** An array of platform ids to restrict this product for. */
Expand Down Expand Up @@ -662,24 +662,24 @@ export interface definitions {
readonly name: definitions["Name"];
readonly type: "boolean" | "string" | "number";
/** This sets whether or not the feature can be customized by a consumer. */
readonly customizable?: boolean;
readonly customizable: boolean;
/**
* This sets whether or not the feature can be upgraded by the consumer after the
* resource has provisioned. Upgrading means setting a higher value or selecting a
* higher element in the list.
*/
readonly upgradable?: boolean;
readonly upgradable: boolean;
/**
* This sets whether or not the feature can be downgraded by the consumer after the
* resource has provisioned. Downgrading means setting a lower value or selecting a
* lower element in the list.
*/
readonly downgradable?: boolean;
readonly downgradable: boolean;
/**
* Sets if this feature’s value is trackable from the provider,
* this only really affects numeric constraints.
*/
readonly measurable?: boolean;
readonly measurable: boolean;
readonly values?: definitions["FeatureValuesList"];
};
/**
Expand All @@ -699,7 +699,7 @@ export interface definitions {
* is selected or is default for the plan.
* Cost is deprecated in favor of the `price.cost` field.
*/
readonly cost?: number;
readonly cost: number;
/**
* Price describes the cost of a feature. It should be preferred over
* the `cost` property.
Expand All @@ -710,13 +710,13 @@ export interface definitions {
* when this value is selected or is default for the plan.
* Number features should use the cost range instead.
*/
readonly cost?: number;
readonly cost: number;
/**
* When a feature is used to multiply the cost of the plan or of
* another feature, multiply factor is used for calculation.
* A feature cannot have both a cost and a multiply factor.
*/
readonly multiply_factor?: number;
readonly multiply_factor: number;
/** Price describes how the feature cost should be calculated. */
readonly formula?: definitions["PriceFormula"];
/** Description explains how a feature is calculated to the user. */
Expand All @@ -737,9 +737,9 @@ export interface definitions {
* means this numeric details has no scale, and will not be or customizable.
* Some plans may not have a measureable or customizable feature.
*/
readonly increment?: number;
readonly increment: number;
/** Minimum value that can be set by a user if customizable */
readonly min?: number;
readonly min: number;
/** Maximum value that can be set by a user if customizable */
readonly max?: number;
/** Applied to the end of the number for display, for example the ‘GB’ in ‘20 GB’. */
Expand All @@ -758,7 +758,7 @@ export interface definitions {
* An integer in 10,000,000ths of cents, will be multiplied by the
* numeric value set in the feature to determine the cost.
*/
readonly cost_multiple?: number;
readonly cost_multiple: number;
};
readonly FeatureValue: {
readonly feature: definitions["Label"];
Expand All @@ -784,40 +784,40 @@ export interface definitions {
* When true, everyone can see the product when requested. When false it will
* not be visible to anyone except those on the provider team.
*/
readonly public?: boolean;
readonly public: boolean;
/**
* When true, the product will be displayed in product listings alongside
* other products. When false the product will be excluded from listings,
* but can still be provisioned directly if it's label is known.
* Any pages that display information about the product when not listed,
* should indicate to webcrawlers that the content should not be indexed.
*/
readonly listed?: boolean;
readonly listed: boolean;
/**
* Object to hold various flags for marketing purposes only. These are values
* that need to be stored, but should not affect decision making in code. If
* we find ourselves in a position where we think they should, we should
* consider refactoring our listing definition.
*/
readonly marketing?: {
readonly marketing: {
/**
* Indicates whether or not the product is in `Beta` and should be
* advertised as such. This does not have any impact on who can access the
* product, it is just used to inform consumers through our clients.
*/
readonly beta?: boolean;
readonly beta: boolean;
/**
* Indicates whether or not the product is in `New` and should be
* advertised as such. This does not have any impact on who can access the
* product, it is just used to inform consumers through our clients.
*/
readonly new?: boolean;
readonly new: boolean;
/**
* Indicates whether or not the product is in `New` and should be
* advertised as such. This does not have any impact on who can access the
* product, it is just used to inform consumers through our clients.
*/
readonly featured?: boolean;
readonly featured: boolean;
};
};
/**
Expand Down Expand Up @@ -858,7 +858,7 @@ export interface definitions {
* * `multiple`: Multiple credentials are supported at the same time.
* * `unknown`: The credential type is unknown.
*/
readonly credential?: "none" | "single" | "multiple" | "unknown";
readonly credential: "none" | "single" | "multiple" | "unknown";
};
readonly ProductBody: {
readonly provider_id: definitions["ID"];
Expand Down
Loading