openapi-ts / openapi-typescript

Generate TypeScript types from OpenAPI 3 specs
https://openapi-ts.dev
MIT License
5.62k stars 453 forks source link

Generated type of additionalProperties values should not include undefined #1267

Closed tankers746 closed 1 year ago

tankers746 commented 1 year ago

Description

Reposting this issue https://github.com/drwpow/openapi-typescript/issues/1070

I strongly believe that the library should generate

type Foo = {
    [index: string]: string
}

instead of

type Foo = {
    [index: string]: string | undefined
}

The reasoning given in the issue above refers to the additional safety provided by the undefined, however this safety is now provided by noUncheckedIndexAccess mentioned here https://github.com/drwpow/openapi-typescript/issues/1070#issuecomment-1500751490

The current setup makes iteration difficult e.g.

const obj: Foo = {};
const values = Object.values(obj); -> string | undefined

When in reality, undefined is not present in JSON and therefore that is not accurate.

Name Version
openapi-typescript 6.4.0
Node.js x.x.x
OS + version macOS 13, Windows 11, etc.

Reproduction

How can this be reproduced / when did the error occur?

Expected result

(in case it’s not obvious)

Checklist

drwpow commented 1 year ago

See also the discussion in #1018. I agree this change is necessary, and we should reverse the original decision to do this. Would welcome a PR on this!

drwpow commented 1 year ago

The new release v6.5.0 features this change

tkrotoff commented 11 months ago

This doesn't make sense when you send data.

Scenario: my API takes some random additional attributes:

additional_attributes?: {
  [key: string]: string;
};

How can I write this:

additional_attributes: {
  brand_name: 'Orao',
  label: 'Kitesurfing kite - Straterial - FREERIDE HANGTIME - 12M2',
  color: hasColor ? color : undefined
},

Properties with undefined values are omitted in JSON.

Should I write this for each optional field to please the types?:

if (hasColor) {
  additional_attributes.color = color;
}
naktinis commented 2 months ago

Possible regression in v7?

tkrotoff commented 2 months ago

This is the patch we use at my company (Decathlon) for openapi-typescript v6.7.3:

diff --git a/dist/transform/schema-object.js b/dist/transform/schema-object.js
index daa4e7c760dd027afdb189fa59a37215e3f71c80..2627cbc310d31e6b8b5fc953a0cac4547ec73a45 100644
--- a/dist/transform/schema-object.js
+++ b/dist/transform/schema-object.js
@@ -161,13 +161,17 @@ export function defaultSchemaObjectTransform(schemaObject, { path, ctx }) {
                     });
                 }
             }
-            const numProperties = schemaObject.properties ? Object.keys(schemaObject.properties).length : 0;
-            if (schemaObject.properties && ((!schemaObject.required && numProperties) || (schemaObject.required && numProperties !== schemaObject.required.length))) {
-                coreType.push(indent(`[key: string]: ${tsUnionOf(addlType ? addlType : "unknown", "undefined")};`, indentLv));
-            }
-            else {
-                coreType.push(indent(`[key: string]: ${addlType ? addlType : "unknown"};`, indentLv));
-            }
+            // FIXME https://github.com/drwpow/openapi-typescript/issues/1267#issuecomment-1771291439
+            // See https://github.com/drwpow/openapi-typescript/pull/1295/files#diff-e84ae76974aeb697c142565b89cf0eeae92f6cc71f8d24447b29b5862c3a628f
+            //
+            // const numProperties = schemaObject.properties ? Object.keys(schemaObject.properties).length : 0;
+            // if (schemaObject.properties && ((!schemaObject.required && numProperties) || (schemaObject.required && numProperties !== schemaObject.required.length))) {
+            //     coreType.push(indent(`[key: string]: ${tsUnionOf(addlType ? addlType : "unknown", "undefined")};`, indentLv));
+            // }
+            // else {
+            //     coreType.push(indent(`[key: string]: ${addlType ? addlType : "unknown"};`, indentLv));
+            // }
+            coreType.push(indent(`[key: string]: ${tsUnionOf(addlType ? addlType : "unknown", "undefined")};`, indentLv));
         }
         if (schemaObject.$defs && typeof schemaObject.$defs === "object" && Object.keys(schemaObject.$defs).length) {
             coreType.push(indent(`$defs: ${transformSchemaObjectMap(schemaObject.$defs, { path: `${path}$defs/`, ctx: { ...ctx, indentLv } })};`, indentLv));