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

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

Open
LlinksRechts opened this issue Dec 14, 2021 · 1 comment
Open

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

LlinksRechts opened this issue Dec 14, 2021 · 1 comment
Labels
enhancement Request for a change or improvement of existing functionality
Projects

Comments

@LlinksRechts
Copy link

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
Copy link
Member

vmasek commented Dec 14, 2021

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

@vmasek vmasek added this to To do in Main board via automation Dec 14, 2021
@vmasek vmasek added the enhancement Request for a change or improvement of existing functionality label Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request for a change or improvement of existing functionality
Projects
Status: To do
Development

No branches or pull requests

2 participants