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

--array-length with minItems: 1 generates empty array #2048

Open
2 tasks
OliverJAsh opened this issue Dec 12, 2024 · 4 comments · Fixed by #2053
Open
2 tasks

--array-length with minItems: 1 generates empty array #2048

OliverJAsh opened this issue Dec 12, 2024 · 4 comments · Fixed by #2053
Assignees
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library

Comments

@OliverJAsh
Copy link

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 OliverJAsh added bug Something isn't working openapi-ts Relevant to the openapi-typescript library labels Dec 12, 2024
duncanbeevers added a commit to duncanbeevers/openapi-typescript that referenced this issue Dec 16, 2024
@duncanbeevers
Copy link
Contributor

duncanbeevers commented Dec 16, 2024

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?

drwpow pushed a commit that referenced this issue Jan 3, 2025
* Simplify minItems / maxItems tuple generation

Closes #2048

* fixup! Simplify minItems / maxItems tuple generation

Account for immutable: true

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation

* fixup! Simplify minItems / maxItems tuple generation
@drwpow drwpow self-assigned this Jan 3, 2025
@drwpow
Copy link
Contributor

drwpow commented Jan 3, 2025

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)

duncanbeevers added a commit to duncanbeevers/openapi-typescript that referenced this issue Jan 3, 2025
@duncanbeevers
Copy link
Contributor

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)

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.

@drwpow
Copy link
Contributor

drwpow commented Jan 6, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants