flowup / api-client-generator

Angular REST API client generator from Swagger YAML or JSON file with camel case settigs
MIT License
115 stars 21 forks source link

Nullable non-top-level response fields - guards #135

Open LlinksRechts opened 2 years ago

LlinksRechts commented 2 years ago

When guards are enabled, there is a check for x-nullable in determineResponseType; however, this only applies only to the outermost object of the response; in reality, fields nested inside of this object could also be marked as x-nullable. In this case, the generated code will throw an error if null is returned in the response.

What I suggest is to add something like the following (works for my use case, but I haven't yet verified this for all possible value types since I wanted to get feedback here before):

diff --git a/src/parser.ts b/src/parser.ts
index bb8bf89..3fe0ea8 100644
--- a/src/parser.ts
+++ b/src/parser.ts
@@ -435,6 +435,9 @@ function parseSchema(
     return { type: 'any', imports: [] };
   }

+  const checkNull = (iterName: string) =>
+    property['x-nullable'] || false ? ` || ${iterName} === null` : '';
+
   if ('enum' in property) {
     const enumValues =
       property.type === 'number'
@@ -451,7 +454,7 @@ function parseSchema(
               name,
               isRequired,
               (iterName: string) =>
-                `[${enumValues.join(', ')}].includes(${iterName})`,
+                `[${enumValues.join(', ')}].includes(${iterName})${checkNull(iterName)}`,
             ),
     };
   }
@@ -466,7 +469,7 @@ function parseSchema(
             guardOptional(
               name,
               isRequired,
-              (iterName: string) => `typeof ${iterName} === 'object'`,
+              (iterName: string) => `typeof ${iterName} === 'object'${checkNull(iterName)}`,
             ),
     }; // type occurrence of inlined properties as object instead of any (TODO: consider supporting inlined properties)
   }
@@ -484,7 +487,7 @@ function parseSchema(
               name,
               isRequired,
               (iterName: string) =>
-                `${prefixGuards ? 'guards.' : ''}is${refType}(${iterName})`,
+                `${prefixGuards ? 'guards.' : ''}is${refType}(${iterName})${checkNull(iterName)}`,
             ),
     };
   }
@@ -504,7 +507,7 @@ function parseSchema(
           ? undefined
           : () =>
               guardOptional(name, isRequired, (iterName: string) =>
-                guardArray(iterName, parsedArrayItemsSchema.guard!),
+                `${guardArray(iterName, parsedArrayItemsSchema.guard!)}${checkNull(iterName)}`,
               ),
     };
   }
@@ -528,8 +531,8 @@ function parseSchema(
           : () =>
               guardOptional(name, isRequired, (iterName: string) =>
                 isJustObject
-                  ? `typeof ${iterName} === 'object'`
-                  : guardDictionary(iterName, parsedDictionarySchema.guard!),
+                  ? `typeof ${iterName} === 'object'${checkNull(iterName)}`
+                  : `${guardDictionary(iterName, parsedDictionarySchema.guard!)}${checkNull(iterName)}`,
               ),
     };
   }
@@ -547,8 +550,8 @@ function parseSchema(
             ? ''
             : guardOptional(name, isRequired, (iterName: string) =>
                 type === 'File'
-                  ? `${iterName} instanceof File`
-                  : `typeof ${iterName} === '${type}'`,
+                  ? `${iterName} instanceof File${checkNull(iterName)}`
+                    : `typeof ${iterName} === '${type}'${checkNull(iterName)}`
               ),
   };
 }
vmasek commented 2 years ago

Thanks for the description and proposal, I'll have a look and try to introduce it in the version :+1: