openapi-typescript icon indicating copy to clipboard operation
openapi-typescript copied to clipboard

`--array-length` with `minItems: 1` generates empty array

Open OliverJAsh opened this issue 1 year ago • 4 comments

Description

--array-length with minItems: 1 generates empty array.

Tested with v7.4.4 and v7.1.2.

Reproduction

Copy the example here: https://openapi-ts.dev/cli#arraylength

components:
  schemas:
    TupleType:
      type: array
      items:
        type: string
      minItems: 1
      maxItems: 2

Expected result

export interface components {
    schemas: {
        readonly TupleType: [string] | [string, string];
    };
}

Actual result

export interface components {
    schemas: {
        readonly TupleType: [
        ] | [
            string
        ];
    };
}

Checklist

OliverJAsh avatar Dec 12 '24 21:12 OliverJAsh

I've been poking at this implementation in order to fix this bug and came across what I think is unexpected behavior, making it impossible to generate heterogeneous array types.

Given this schema, here are the generated types.

components:
  schemas:
    HeterogeneousArray:
      type: array
      items:
        - type: number
        - type: string
// Result type is [number, string], not [number | string]()
interface components = {
  schemas: {
    HeterogeneousArray: [number, string];
  }
};

Forgive me if I missed some discussion around this, but I thought for items in fixed positions it would be necessary to use the prefixItems portion of the OpenAPI schema, and that the primary items slot could be used to specify heterogeneous array member types.

Looking at the spec, items shouldn't be able to be an array at all.

# Incorrect
items:
  - type: string
  - type: integer

# Incorrect as well
items:
  type:
    - string
    - integer

Is this array-of-item-types a custom behavior only supported by openapi-typescript?

duncanbeevers avatar Dec 16 '24 18:12 duncanbeevers

Yeah I’m seeing the same thing with minItems + maxItems—empty array generation. The original bug has readonly but I’m seeing it without that flag (not that that is likely to cause an issue, but just as a detail, sometimes readonly can confuse the TS AST because it’s a really strange API).

This looks like a bug to me; would love a PR to fix this 🙏

Looking at the spec, items shouldn't be able to be an array at all.

@duncanbeevers this is one of those “rabbit hole” things where because OpenAPI 3.x supports JSON Schema, and the 2020-12 version supports this, by extension it’s technically allowed. It’s strange, but there is some evidence to suggest this is valid, believe it or not (also #1011 was one of the issues that led to the deeper rewrite of 7.x)

drwpow avatar Jan 03 '25 23:01 drwpow

It’s strange, but there is some evidence to suggest this is valid, believe it or not (also https://github.com/openapi-ts/openapi-typescript/issues/1011 was one of the issues that led to the deeper rewrite of 7.x)

Okay. I'll re-add support for items-as-array with the prefixItems behavior.

Is there already a mechanism in place for deprecations? It sounds like 8.x will have a lot of breaking changes, but I think it's probably possible to keep the old + new behaviors, putting the new stuff behind flags, and logging deprecation warnings when the old behaviors are still used.

duncanbeevers avatar Jan 05 '25 22:01 duncanbeevers

I think it's probably possible to keep the old + new behaviors, putting the new stuff behind flags

To be honest I’ve never thought of that use of feature flags for this library. We’ve used them for completely alternate behaviors that are preference, not necessarily breaking changes. But I do know some libraries do that, especially if something is extremely disruptive and needs feedback and testing.

That may also be easier to maintain than a separate branch/release tag that desyncs with 7.x, where it fixes some bugs but is otherwise behind all the patches and features. Will run this idea by the other maintainers for feedback

drwpow avatar Jan 06 '25 03:01 drwpow