fix: handle references to schemas with illegal names

hey-api · Jul 17, 2024 · 20759e2 · 20759e2
1 parent 274851b
commit 20759e2
Show file tree

Hide file tree

Showing 41 changed files with 256 additions and 108 deletions.
diff --git a/.changeset/old-islands-raise.md b/.changeset/old-islands-raise.md
@@ -0,0 +1,5 @@
+---
+'@hey-api/openapi-ts': patch
+---
+
+fix: handle references to schemas with illegal names
diff --git a/packages/openapi-ts/src/openApi/common/parser/type.ts b/packages/openapi-ts/src/openApi/common/parser/type.ts
@@ -1,6 +1,7 @@
 import camelcase from 'camelcase';
 
 import { getConfig, isStandaloneClient } from '../../../utils/config';
+import { refParametersPartial } from '../../../utils/const';
 import { reservedWordsRegExp } from '../../../utils/reservedWords';
 import { transformTypeName } from '../../../utils/transform';
 import { isDefinitionTypeNullable } from '../../v3/parser/inferType';
@@ -136,7 +137,7 @@ export const getType = ({
     let encodedType = transformTypeName(
       ensureValidTypeScriptJavaScriptIdentifier(typeWithoutNamespace),
     );
-    if (type.startsWith('#/components/parameters/')) {
+    if (type.startsWith(refParametersPartial)) {
       // prefix parameter names to avoid conflicts, assuming people are mostly
       // interested in importing schema types and don't care about this naming
       encodedType = `Parameter${encodedType}`;
@@ -166,5 +167,6 @@ export const transformTypeKeyName = (value: string): string => {
   }
 
   const clean = sanitizeOperationParameterName(value).trim();
-  return camelcase(clean).replace(reservedWordsRegExp, '_$1');
+  const name = camelcase(clean).replace(reservedWordsRegExp, '_$1');
+  return name;
 };
diff --git a/packages/openapi-ts/src/openApi/v3/parser/getModelComposition.ts b/packages/openapi-ts/src/openApi/v3/parser/getModelComposition.ts
@@ -62,14 +62,15 @@ export const getModelComposition = ({
   let properties: Model[] = [];
 
   definitions
-    .map((def) =>
-      getModel({
+    .map((def) => {
+      const modelFromDef = getModel({
         definition: def,
         openApi,
         parentDefinition: definition,
         types,
-      }),
-    )
+      });
+      return modelFromDef;
+    })
     .forEach((model) => {
       composition.$refs = [...composition.$refs, ...model.$refs];
       composition.imports = [...composition.imports, ...model.imports];

diff --git a/packages/openapi-ts/src/openApi/v3/parser/getModels.ts b/packages/openapi-ts/src/openApi/v3/parser/getModels.ts
@@ -1,7 +1,6 @@
 import type { Client } from '../../../types/client';
 import { getConfig } from '../../../utils/config';
-import { reservedWordsRegExp } from '../../../utils/reservedWords';
-import { getType } from '../../common/parser/type';
+import { getParametersMeta, getSchemasMeta } from '../../../utils/meta';
 import type { OpenApi } from '../interfaces/OpenApi';
 import { getModel } from './getModel';
 import { getParameterSchema } from './parameter';
@@ -23,13 +22,8 @@ export const getModels = (
 
   Object.entries(openApi.components.schemas ?? {}).forEach(
     ([definitionName, definition]) => {
-      const definitionType = getType({ type: definitionName });
-      const name = definitionType.base.replace(reservedWordsRegExp, '_$1');
-      const meta = {
-        $ref: `#/components/schemas/${definitionName}`,
-        name,
-      };
-      types[name] = meta;
+      const meta = getSchemasMeta(definitionName);
+      types[meta.name] = meta;
       const model = getModel({
         definition,
         isDefinition: true,
@@ -51,27 +45,8 @@ export const getModels = (
         return;
       }
 
-      const definitionType = getType({ type: definitionName });
-      /**
-       * Prefix parameter names to avoid name conflicts with schemas.
-       * Assuming people are mostly interested in importing schema types
-       * and don't care about this name as much. It should be resolved in
-       * a cleaner way, there just isn't a good deduplication strategy
-       * today. This is a workaround in the meantime, hopefully reducing
-       * the chance of conflicts.
-       *
-       * Example where this would break: schema named `ParameterFoo` and
-       * parameter named `Foo` (this would transform to `ParameterFoo`)
-       *
-       * Note: there's a related code to this workaround in `getType()`
-       * method that needs to be cleaned up when this is addressed.
-       */
-      const name = `Parameter${definitionType.base.replace(reservedWordsRegExp, '_$1')}`;
-      const meta = {
-        $ref: `#/components/parameters/${definitionName}`,
-        name,
-      };
-      types[name] = meta;
+      const meta = getParametersMeta(definitionName);
+      types[meta.name] = meta;
       const model = getModel({
         definition: schema,
         isDefinition: true,

diff --git a/packages/openapi-ts/src/openApi/v3/parser/getOperationParameter.ts b/packages/openapi-ts/src/openApi/v3/parser/getOperationParameter.ts
@@ -1,5 +1,6 @@
 import type { Client } from '../../../types/client';
 import { getConfig, isStandaloneClient } from '../../../utils/config';
+import { refParametersPartial } from '../../../utils/const';
 import { enumMeta } from '../../../utils/enum';
 import type { OperationParameter } from '../../common/interfaces/client';
 import { getDefault } from '../../common/parser/getDefault';
@@ -71,7 +72,7 @@ export const getOperationParameter = ({
 
   let schema = getParameterSchema(parameter);
   if (schema) {
-    if (schema.$ref?.startsWith('#/components/parameters/')) {
+    if (schema.$ref?.startsWith(refParametersPartial)) {
       schema = getRef<OpenApiSchema>(openApi, schema);
     }
 

diff --git a/packages/openapi-ts/src/utils/const.ts b/packages/openapi-ts/src/utils/const.ts
@@ -0,0 +1,3 @@
+export const refParametersPartial = '#/components/parameters/';
+
+export const refSchemasPartial = '#/components/schemas/';
diff --git a/packages/openapi-ts/src/utils/meta.ts b/packages/openapi-ts/src/utils/meta.ts
@@ -0,0 +1,37 @@
+import { getType } from '../openApi/common/parser/type';
+import { refParametersPartial, refSchemasPartial } from './const';
+import { reservedWordsRegExp } from './reservedWords';
+
+export const getParametersMeta = (definitionName: string) => {
+  const definitionType = getType({ type: definitionName });
+  /**
+   * Prefix parameter names to avoid name conflicts with schemas.
+   * Assuming people are mostly interested in importing schema types
+   * and don't care about this name as much. It should be resolved in
+   * a cleaner way, there just isn't a good deduplication strategy
+   * today. This is a workaround in the meantime, hopefully reducing
+   * the chance of conflicts.
+   *
+   * Example where this would break: schema named `ParameterFoo` and
+   * parameter named `Foo` (this would transform to `ParameterFoo`)
+   *
+   * Note: there's a related code to this workaround in `getType()`
+   * method that needs to be cleaned up when this is addressed.
+   */
+  const name = `Parameter${definitionType.base.replace(reservedWordsRegExp, '_$1')}`;
+  const meta = {
+    $ref: refParametersPartial + definitionName,
+    name,
+  };
+  return meta;
+};
+
+export const getSchemasMeta = (definitionName: string) => {
+  const definitionType = getType({ type: definitionName });
+  const name = definitionType.base.replace(reservedWordsRegExp, '_$1');
+  const meta = {
+    $ref: refSchemasPartial + definitionName,
+    name,
+  };
+  return meta;
+};
diff --git a/packages/openapi-ts/src/utils/type.ts b/packages/openapi-ts/src/utils/type.ts
@@ -3,8 +3,10 @@ import type { Model } from '../openApi';
 import { transformTypeKeyName } from '../openApi/common/parser/type';
 import type { Client } from '../types/client';
 import { getConfig, isStandaloneClient } from './config';
+import { refSchemasPartial } from './const';
 import { enumValue } from './enum';
 import { escapeComment, escapeName, unescapeName } from './escape';
+import { getSchemasMeta } from './meta';
 import { unique } from './unique';
 
 const base = (model: Model) => {
@@ -27,7 +29,20 @@ const base = (model: Model) => {
 const typeReference = (model: Model) => {
   // nullable is false when base is null to avoid duplicate null statements
   const isNullable = model.base === 'null' ? false : model.isNullable;
-  const unionNode = compiler.typedef.union([base(model)], isNullable);
+  let typeNode = base(model);
+  /**
+   * special handling for single reference. The current approach didn't handle
+   * transformed names, this fixes that. We should add a more robust solution,
+   * but this will work for now.
+   * {@link https://github.com/hey-api/openapi-ts/issues/768}
+   */
+  if (model.export === 'reference' && model.$refs.length === 1) {
+    if (model.$refs[0].startsWith(refSchemasPartial)) {
+      const meta = getSchemasMeta(model.base);
+      typeNode = compiler.typedef.basic(meta.name);
+    }
+  }
+  const unionNode = compiler.typedef.union([typeNode], isNullable);
   return unionNode;
 };
 

diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3/schemas.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3/schemas.gen.ts.snap
@@ -1637,7 +1637,7 @@ export const $ModelWithAnyOfConstantSizeArrayWithNSizeAndOptions = {
                 type: 'number'
             },
             {
-                type: 'string'
+                '$ref': '#/components/schemas/import'
             }
         ]
     },
@@ -1758,4 +1758,9 @@ export const $DeleteFooData = {
 export const $DeleteFooData2 = {
     description: 'Model used to test deduplication strategy',
     type: 'string'
+} as const;
+
+export const $import = {
+    description: 'Model with restricted keyword name',
+    type: 'string'
 } as const;
diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3/services.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3/services.gen.ts.snap
@@ -509,7 +509,7 @@ export class ResponseService {
     }
 
     /**
-     * @returns ModelWithString
+     * @returns import
      * @throws ApiError
      */
     public static callWithResponse(): CancelablePromise<CallWithResponseResponse> {

diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3/types.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3/types.gen.ts.snap
@@ -887,8 +887,8 @@ export type ModelWithAnyOfConstantSizeArrayNullable = [
 ];
 
 export type ModelWithAnyOfConstantSizeArrayWithNSizeAndOptions = [
-    number | string,
-    number | string
+    number | _import,
+    number | _import
 ];
 
 export type ModelWithAnyOfConstantSizeArrayAndIntersect = [
@@ -987,6 +987,11 @@ export type DeleteFooData = string;
  */
 export type DeleteFooData2 = string;
 
+/**
+ * Model with restricted keyword name
+ */
+export type _import = string;
+
 /**
  * This is a reusable parameter
  */
@@ -1255,7 +1260,7 @@ export type CallWithNoContentResponseResponse = void;
 
 export type CallWithResponseAndNoContentResponseResponse = number | void;
 
-export type CallWithResponseResponse = ModelWithString;
+export type CallWithResponseResponse = _import;
 
 export type CallWithDuplicateResponsesResponse = ModelWithBoolean & ModelWithInteger | ModelWithString;
 
@@ -1530,7 +1535,7 @@ export type $OpenApiTs = {
     '/api/v{api-version}/response': {
         get: {
             res: {
-                default: ModelWithString;
+                default: _import;
             };
         };
         post: {

diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3_angular/schemas.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3_angular/schemas.gen.ts.snap
@@ -1637,7 +1637,7 @@ export const $ModelWithAnyOfConstantSizeArrayWithNSizeAndOptions = {
                 type: 'number'
             },
             {
-                type: 'string'
+                '$ref': '#/components/schemas/import'
             }
         ]
     },
@@ -1758,4 +1758,9 @@ export const $DeleteFooData = {
 export const $DeleteFooData2 = {
     description: 'Model used to test deduplication strategy',
     type: 'string'
+} as const;
+
+export const $import = {
+    description: 'Model with restricted keyword name',
+    type: 'string'
 } as const;
diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3_angular/services.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3_angular/services.gen.ts.snap
@@ -566,7 +566,7 @@ export class ResponseService {
     }
 
     /**
-     * @returns ModelWithString
+     * @returns import
      * @throws ApiError
      */
     public callWithResponse(): Observable<CallWithResponseResponse> {

diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3_angular/types.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3_angular/types.gen.ts.snap
@@ -777,8 +777,8 @@ export type ModelWithAnyOfConstantSizeArrayNullable = [
 ];
 
 export type ModelWithAnyOfConstantSizeArrayWithNSizeAndOptions = [
-    number | string,
-    number | string
+    number | _import,
+    number | _import
 ];
 
 export type ModelWithAnyOfConstantSizeArrayAndIntersect = [
@@ -864,6 +864,11 @@ export type DeleteFooData = string;
  */
 export type DeleteFooData2 = string;
 
+/**
+ * Model with restricted keyword name
+ */
+export type _import = string;
+
 /**
  * This is a reusable parameter
  */
@@ -1132,7 +1137,7 @@ export type CallWithNoContentResponseResponse = void;
 
 export type CallWithResponseAndNoContentResponseResponse = number | void;
 
-export type CallWithResponseResponse = ModelWithString;
+export type CallWithResponseResponse = _import;
 
 export type CallWithDuplicateResponsesResponse = ModelWithBoolean & ModelWithInteger | ModelWithString;
 
@@ -1407,7 +1412,7 @@ export type $OpenApiTs = {
     '/api/v{api-version}/response': {
         get: {
             res: {
-                default: ModelWithString;
+                default: _import;
             };
         };
         post: {

diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3_axios/schemas.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3_axios/schemas.gen.ts.snap
@@ -1637,7 +1637,7 @@ export const $ModelWithAnyOfConstantSizeArrayWithNSizeAndOptions = {
                 type: 'number'
             },
             {
-                type: 'string'
+                '$ref': '#/components/schemas/import'
             }
         ]
     },
@@ -1758,4 +1758,9 @@ export const $DeleteFooData = {
 export const $DeleteFooData2 = {
     description: 'Model used to test deduplication strategy',
     type: 'string'
+} as const;
+
+export const $import = {
+    description: 'Model with restricted keyword name',
+    type: 'string'
 } as const;
diff --git a/packages/openapi-ts/test/__snapshots__/test/generated/v3_axios/services.gen.ts.snap b/packages/openapi-ts/test/__snapshots__/test/generated/v3_axios/services.gen.ts.snap
@@ -509,7 +509,7 @@ export class ResponseService {
     }
 
     /**
-     * @returns ModelWithString
+     * @returns import
      * @throws ApiError
      */
     public static callWithResponse(): CancelablePromise<CallWithResponseResponse> {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		export const refParametersPartial = '#/components/parameters/';

		export const refSchemasPartial = '#/components/schemas/';