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

De-sugaring Type Aliases with arrays #1460

Open
ReidWeb opened this issue Jun 14, 2024 · 4 comments
Open

De-sugaring Type Aliases with arrays #1460

ReidWeb opened this issue Jun 14, 2024 · 4 comments
Labels
bug This issue is a bug. p2

Comments

@ReidWeb
Copy link

ReidWeb commented Jun 14, 2024

Having reviewed the docs it my understanding that JSII/JSII-Docgen de-sugars type aliases are de-sugared, this is a perfectly understandable design decision.

However I've come across a bit of a quirky behaviour as described below, and as is viewable in the minimum reproducible example repo I created.

/**
 * Interface for providing config for paramOne
 */
export interface IOne {
    /**
     * Param foo
     */
    readonly foo: string
}

/**
 * Declared type to wrap two options user can provide for the param
 */
export type ParamType = string | IOne


/**
 * Function parameters
 */
export interface FunctionParams {
    readonly paramOne: ParamType[]
}

export class ConstructFoo {

    readonly internalParam: ParamType[]

    constructor(fnParams: FunctionParams) {
        this.internalParam = fnParams.paramOne
    }
}

As you can see, I am declaring parameter paramOne to expect an array of objects, I am not explicitly stating the type of the array as would be the case if I declared string[] | IOne[], thus from a TypeScript perspective, it will allow users to pass a mixed array of strings and objects matching interface IOne, as is shown to be working in my example invocation.

Current Behaviour

However the docs come out as

```typescript
public readonly paramOne: string | IOne[];
```

Which implies only the second type in the union can be an array, if a user were to attempt to use this as described in the docs they would get invalid code.

const arrayParams : ParamType[]  = [ // ✅ Valid TypeScript Code
    {
      foo: 'bar'
    },
    'foobar'
]

const asDocsDescribe: string | IOne[]  = [ // 🚨 Not valid code!
    {
        foo: 'bar'
    },
    'foobar'
]

I believe this is an unintended consequence of the de-sugaring you are performing, and may require remediation?

Expected behaviour

Docgen should produce doc declaration for type paramOne as follows

```typescript
public readonly paramOne: string[] | IOne[];
```

Background

I am using this in a way where I want objects of interface IOne or strings in the interface, not both. I had declared my code as such as I found there were more optimal type guards available to me when I did it this way vs having string[] | IOne[]

@mrgrain
Copy link
Contributor

mrgrain commented Jul 1, 2024

Relevant part of the jsii assembly:

        {
          "abstract": true,
          "docs": {
            "stability": "stable"
          },
          "immutable": true,
          "locationInModule": {
            "filename": "lib/ConstructFoo.ts",
            "line": 21
          },
          "name": "paramOne",
          "type": {
            "collection": {
              "elementtype": {
                "union": {
                  "types": [
                    {
                      "primitive": "string"
                    },
                    {
                      "fqn": "jsii-docgen-mre.IOne"
                    }
                  ]
                }
              },
              "kind": "array"
            }
          }
        },

@mrgrain
Copy link
Contributor

mrgrain commented Jul 1, 2024

Docgen should produce doc declaration for type paramOne as follows

I'd even go further and say the type should be (which is what the jsii assembly says):

```typescript
public readonly paramOne: Array<string | IOne>;
```

@mrgrain mrgrain added the p2 label Jul 1, 2024
@mrgrain mrgrain changed the title Potential issue: De-sugaring Type Aliases with arrays De-sugaring Type Aliases with arrays Jul 1, 2024
@mrgrain mrgrain added the bug This issue is a bug. label Jul 1, 2024
@ReidWeb
Copy link
Author

ReidWeb commented Jul 1, 2024

Yup, makes sense to me!

@mrgrain
Copy link
Contributor

mrgrain commented Jul 1, 2024

Labelling this as a p2 bug. jsii-docgen is a community led project with fixes from the maintainers mainly being provided as we need them for our own projects. This means it's unlikely we'll get to this issue anytime soon.

Contributions are welcomed and we aim to respond to all PRs as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2
Projects
None yet
Development

No branches or pull requests

2 participants