Skip to content

Commit

Permalink
Merge pull request #538 from hey-api/fix/dedup-exported-types
Browse files Browse the repository at this point in the history
fix: deduplicate exported data and response types
  • Loading branch information
mrlubos authored May 6, 2024
2 parents afa3bd6 + c9cf5da commit e73df34
Show file tree
Hide file tree
Showing 51 changed files with 818 additions and 333 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-pots-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@hey-api/openapi-ts": patch
---

fix: deduplicate exported data and response types
24 changes: 14 additions & 10 deletions packages/openapi-ts/src/compiler/typedef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,28 @@ export const createTypeNode = (

/**
* Create a type alias declaration. Example `export type X = Y;`.
* @param name - the name of the type.
* @param type - the type.
* @param comments - comments to add if any.
* @param comment (optional) comments to add
* @param name the name of the type
* @param type the type
* @returns ts.TypeAliasDeclaration
*/
export const createTypeAliasDeclaration = (
name: string,
type: string | ts.TypeNode,
comments?: Comments,
): ts.TypeAliasDeclaration => {
export const createTypeAliasDeclaration = ({
comment,
name,
type,
}: {
comment?: Comments;
name: string;
type: string | ts.TypeNode;
}): ts.TypeAliasDeclaration => {
const node = ts.factory.createTypeAliasDeclaration(
[ts.factory.createModifier(ts.SyntaxKind.ExportKeyword)],
ts.factory.createIdentifier(name),
[],
createTypeNode(type),
);
if (comments) {
addLeadingJSDocComment(node, comments);
if (comment) {
addLeadingJSDocComment(node, comment);
}
return node;
};
Expand Down
15 changes: 15 additions & 0 deletions packages/openapi-ts/src/openApi/common/interfaces/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,17 @@ export interface Schema {
uniqueItems?: boolean;
}

export interface ModelMeta {
/**
* Ref to the type in OpenAPI specification.
*/
$ref: string;
/**
* Name passed to the initial `getModel()` call.
*/
name: string;
}

export interface Model extends Schema {
/**
* **Experimental.** Contains list of original refs so they can be used
Expand All @@ -109,6 +120,10 @@ export interface Model extends Schema {
| 'reference';
imports: string[];
link: Model | null;
meta?: ModelMeta;
/**
* @deprecated use `meta.name` instead
*/
name: string;
properties: Model[];
template: string | null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';

import type { Config } from '../../../../types/config';
import { getMappedType, getType } from '../type';

vi.mock('../../../../utils/config', () => {
const config: Partial<Config> = {
types: {},
};
return {
getConfig: () => config,
};
});

describe('getMappedType', () => {
it.each([
{ expected: undefined, type: '' },
Expand Down Expand Up @@ -32,7 +42,7 @@ describe('getMappedType', () => {

describe('getType', () => {
it('should convert int', () => {
const type = getType('int');
const type = getType({ type: 'int' });
expect(type.type).toEqual('number');
expect(type.base).toEqual('number');
expect(type.template).toEqual(null);
Expand All @@ -41,7 +51,7 @@ describe('getType', () => {
});

it('should convert string', () => {
const type = getType('string');
const type = getType({ type: 'string' });
expect(type.type).toEqual('string');
expect(type.base).toEqual('string');
expect(type.template).toEqual(null);
Expand All @@ -50,7 +60,7 @@ describe('getType', () => {
});

it('should convert string array', () => {
const type = getType('array[string]');
const type = getType({ type: 'array[string]' });
expect(type.type).toEqual('string[]');
expect(type.base).toEqual('string');
expect(type.template).toEqual(null);
Expand All @@ -59,7 +69,7 @@ describe('getType', () => {
});

it('should convert template with primary', () => {
const type = getType('#/components/schemas/Link[string]');
const type = getType({ type: '#/components/schemas/Link[string]' });
expect(type.type).toEqual('Link<string>');
expect(type.base).toEqual('Link');
expect(type.template).toEqual('string');
Expand All @@ -68,7 +78,7 @@ describe('getType', () => {
});

it('should convert template with model', () => {
const type = getType('#/components/schemas/Link[Model]');
const type = getType({ type: '#/components/schemas/Link[Model]' });
expect(type.type).toEqual('Link<Model>');
expect(type.base).toEqual('Link');
expect(type.template).toEqual('Model');
Expand All @@ -77,7 +87,7 @@ describe('getType', () => {
});

it('should have double imports', () => {
const type = getType('#/components/schemas/Link[Link]');
const type = getType({ type: '#/components/schemas/Link[Link]' });
expect(type.type).toEqual('Link<Link>');
expect(type.base).toEqual('Link');
expect(type.template).toEqual('Link');
Expand All @@ -86,7 +96,7 @@ describe('getType', () => {
});

it('should support dot', () => {
const type = getType('#/components/schemas/model.000');
const type = getType({ type: '#/components/schemas/model.000' });
expect(type.type).toEqual('model_000');
expect(type.base).toEqual('model_000');
expect(type.template).toEqual(null);
Expand All @@ -95,7 +105,7 @@ describe('getType', () => {
});

it('should support dashes', () => {
const type = getType('#/components/schemas/some_special-schema');
const type = getType({ type: '#/components/schemas/some_special-schema' });
expect(type.type).toEqual('some_special_schema');
expect(type.base).toEqual('some_special_schema');
expect(type.template).toEqual(null);
Expand All @@ -104,7 +114,7 @@ describe('getType', () => {
});

it('should support dollar sign', () => {
const type = getType('#/components/schemas/$some+special+schema');
const type = getType({ type: '#/components/schemas/$some+special+schema' });
expect(type.type).toEqual('$some_special_schema');
expect(type.base).toEqual('$some_special_schema');
expect(type.template).toEqual(null);
Expand All @@ -113,7 +123,7 @@ describe('getType', () => {
});

it('should support multiple base types', () => {
const type = getType(['string', 'int']);
const type = getType({ type: ['string', 'int'] });
expect(type.type).toEqual('string | number');
expect(type.base).toEqual('string | number');
expect(type.template).toEqual(null);
Expand All @@ -122,7 +132,7 @@ describe('getType', () => {
});

it('should support multiple nullable types', () => {
const type = getType(['string', 'null']);
const type = getType({ type: ['string', 'null'] });
expect(type.type).toEqual('string');
expect(type.base).toEqual('string');
expect(type.template).toEqual(null);
Expand Down
29 changes: 17 additions & 12 deletions packages/openapi-ts/src/openApi/common/parser/type.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { transformTypeName } from '../../../utils/transform';
import { isDefinitionTypeNullable } from '../../v3/parser/inferType';
import type { Type } from '../interfaces/Type';
import { ensureValidTypeScriptJavaScriptIdentifier } from './sanitize';
Expand Down Expand Up @@ -50,10 +51,13 @@ export const getMappedType = (
* @param type String or String[] value like "integer", "Link[Model]" or ["string", "null"].
* @param format String value like "binary" or "date".
*/
export const getType = (
type: string | string[] = 'unknown',
format?: string,
): Type => {
export const getType = ({
format,
type = 'unknown',
}: {
format?: string;
type?: string | string[];
}): Type => {
const result: Type = {
$refs: [],
base: 'unknown',
Expand Down Expand Up @@ -89,12 +93,12 @@ export const getType = (
if (/\[.*\]$/g.test(typeWithoutNamespace)) {
const matches = typeWithoutNamespace.match(/(.*?)\[(.*)\]$/);
if (matches?.length) {
const match1 = getType(
ensureValidTypeScriptJavaScriptIdentifier(matches[1]),
);
const match2 = getType(
ensureValidTypeScriptJavaScriptIdentifier(matches[2]),
);
const match1 = getType({
type: ensureValidTypeScriptJavaScriptIdentifier(matches[1]),
});
const match2 = getType({
type: ensureValidTypeScriptJavaScriptIdentifier(matches[2]),
});

if (match1.type === 'unknown[]') {
result.type = `${match2.type}[]`;
Expand Down Expand Up @@ -122,8 +126,9 @@ export const getType = (
}

if (typeWithoutNamespace) {
let encodedType =
ensureValidTypeScriptJavaScriptIdentifier(typeWithoutNamespace);
let encodedType = transformTypeName(
ensureValidTypeScriptJavaScriptIdentifier(typeWithoutNamespace),
);
if (type.startsWith('#/components/parameters/')) {
// prefix parameter names to avoid conflicts, assuming people are mostly
// interested in importing schema types and don't care about this naming
Expand Down
3 changes: 1 addition & 2 deletions packages/openapi-ts/src/openApi/v2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ export const parse = (openApi: OpenApi): Client => {
const services = getServices(openApi);

return {
enumNames: [],
models,
server,
serviceTypes: [],
services,
types: {},
version,
};
};
45 changes: 27 additions & 18 deletions packages/openapi-ts/src/openApi/v2/parser/getModel.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Model } from '../../common/interfaces/client';
import type { Model, ModelMeta } from '../../common/interfaces/client';
import { getEnums } from '../../common/parser/getEnums';
import { getPattern } from '../../common/parser/getPattern';
import { getType } from '../../common/parser/type';
Expand All @@ -7,12 +7,17 @@ import type { OpenApiSchema } from '../interfaces/OpenApiSchema';
import { getModelComposition } from './getModelComposition';
import { getModelProperties } from './getModelProperties';

export const getModel = (
openApi: OpenApi,
definition: OpenApiSchema,
isDefinition: boolean = false,
name: string = '',
): Model => {
export const getModel = ({
definition,
isDefinition = false,
meta,
openApi,
}: {
definition: OpenApiSchema;
isDefinition?: boolean;
meta?: ModelMeta;
openApi: OpenApi;
}): Model => {
const model: Model = {
$refs: [],
base: 'unknown',
Expand All @@ -33,12 +38,13 @@ export const getModel = (
maxLength: definition.maxLength,
maxProperties: definition.maxProperties,
maximum: definition.maximum,
meta,
minItems: definition.minItems,
minLength: definition.minLength,
minProperties: definition.minProperties,
minimum: definition.minimum,
multipleOf: definition.multipleOf,
name,
name: meta?.name ?? '',
pattern: getPattern(definition.pattern),
properties: [],
template: null,
Expand All @@ -47,7 +53,7 @@ export const getModel = (
};

if (definition.$ref) {
const definitionRef = getType(definition.$ref);
const definitionRef = getType({ type: definition.$ref });
model.export = 'reference';
model.type = definitionRef.type;
model.base = definitionRef.base;
Expand All @@ -69,15 +75,15 @@ export const getModel = (

if (definition.type === 'array' && definition.items) {
if (definition.items.$ref) {
const arrayItems = getType(definition.items.$ref);
const arrayItems = getType({ type: definition.items.$ref });
model.export = 'array';
model.type = arrayItems.type;
model.base = arrayItems.base;
model.template = arrayItems.template;
model.imports.push(...arrayItems.imports);
return model;
} else {
const arrayItems = getModel(openApi, definition.items);
const arrayItems = getModel({ definition: definition.items, openApi });
model.export = 'array';
model.type = arrayItems.type;
model.base = arrayItems.base;
Expand All @@ -93,20 +99,20 @@ export const getModel = (
typeof definition.additionalProperties === 'object'
) {
if (definition.additionalProperties.$ref) {
const additionalProperties = getType(
definition.additionalProperties.$ref,
);
const additionalProperties = getType({
type: definition.additionalProperties.$ref,
});
model.export = 'dictionary';
model.type = additionalProperties.type;
model.base = additionalProperties.base;
model.template = additionalProperties.template;
model.imports.push(...additionalProperties.imports);
return model;
} else {
const additionalProperties = getModel(
const additionalProperties = getModel({
definition: definition.additionalProperties,
openApi,
definition.additionalProperties,
);
});
model.export = 'dictionary';
model.type = additionalProperties.type;
model.base = additionalProperties.base;
Expand Down Expand Up @@ -153,7 +159,10 @@ export const getModel = (

// If the schema has a type than it can be a basic or generic type.
if (definition.type) {
const definitionType = getType(definition.type, definition.format);
const definitionType = getType({
format: definition.format,
type: definition.type,
});
model.export = 'generic';
model.type = definitionType.type;
model.base = definitionType.base;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const getModelComposition = (
const properties: Model[] = [];

definitions
.map((definition) => getModel(openApi, definition))
.map((definition) => getModel({ definition, openApi }))
.filter((model) => {
const hasProperties = model.properties.length;
const hasEnums = model.enums.length;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const getModelProperties = (
([propertyName, property]) => {
const propertyRequired = !!definition.required?.includes(propertyName);
if (property.$ref) {
const model = getType(property.$ref);
const model = getType({ type: property.$ref });
models.push({
$refs: [],
base: model.base,
Expand Down Expand Up @@ -54,7 +54,7 @@ export const getModelProperties = (
uniqueItems: property.uniqueItems,
});
} else {
const model = getModel(openApi, property);
const model = getModel({ definition: property, openApi });
models.push({
$refs: [],
base: model.base,
Expand Down
Loading

0 comments on commit e73df34

Please sign in to comment.