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

[TypeScript] Add support for nested arrays (array of string arrays) #4549

Closed
joebalancioLP opened this issue Apr 25, 2024 · 5 comments · Fixed by #4606
Closed

[TypeScript] Add support for nested arrays (array of string arrays) #4549

joebalancioLP opened this issue Apr 25, 2024 · 5 comments · Fixed by #4606
Assignees
Labels
generator Issues or improvements relater to generation capabilities. type:bug A broken experience
Milestone

Comments

@joebalancioLP
Copy link

Hey there. Running into an issue with the generated client. I have a schema where one of the properties is an array of string arrays. It is returned in an API response and it looks like the following:

      "Foo": {
        "type": "object",
        "properties": {
          "sortBy": {
            "type": "array",
            "items": {
              "type": "array",
              "items": {
                "type": "string"
              }
            }
          }
        }
      },

When I make an API call using the generated client, I get this error:

Error: encountered an unknown type during deserialization object

The stacktrace points to the generated models. When I inspect the generated model, I see that the a few things are off.

The code fails during deserialization in this function. Looks like the wrong function is invoked getCollectionOfPrimitiveValues

export function deserializeIntoFoo(foo: Partial<Foo> | undefined = {}) : Record<string, (node: ParseNode) => void> {
    return {
        "sortBy": n => { foo.sortBy = n.getCollectionOfPrimitiveValues<string>(); },
    }
}

The schema is typed incorrectly. It should be string[][]

export interface Foo extends Parsable {
    /**
     * The sortBy property
     */
    sortBy?: string[];
}

Also I changed the OpenAPI spec a few different ways and here are my findings.

  1. Untyped the nested array. Same error. The property is typed as UntypedNode[]
  2. Changed the nested array to object type. No error, but in the response the array value is parsed as an object { "0": "updatedAt", "1": "DESC" }
  3. Removed properties from Foo. No error, but Foo is no longer typed correctly.
@github-project-automation github-project-automation bot moved this to Todo 📃 in Kiota Apr 25, 2024
@baywet
Copy link
Member

baywet commented Apr 26, 2024

Hi @joebalancioLP
Thanks for using kiota and for reaching out.

General context: multi-dimensional arrays are generally not supported for generation, meaning that kiota will never generate foo: string[][]. This because it'd either require to start generating overly complex orchestration code with loops and whatnot for serialization or require many more methods in the parsenode/serializationwriter interfaces.

I'd have expected kiota to generate foo: UntypedNode[] here, and at runtime the serialization to manage to return UntypedArray[UntypeArray[string]].

Have you tried generating other languages (CSharp/Go/Java) to see if you get a different result in the generated code? (not asking to run it for now)

@andrueastman what's your opinion here?

@baywet baywet added type:bug A broken experience TypeScript Pull requests that update Javascript code Needs: Author Feedback labels Apr 26, 2024
@baywet baywet added this to the Backlog milestone Apr 26, 2024
@andrueastman
Copy link
Member

You're right, I think this is an edge case the KiotaBuilder should probably be updated to handle as an unknown type. I suspect it keeps trying to unwrap the type for the array to come up with type annotation.

Ideally in this scenario, this should be generated as either

  • UntypedNode (not UntypedNode [] as this is the essentially an UntypedArray). This should work out out the box with no serialization changes as the seriliazer will give us a UntypedArray[UntypeArray[string]] at runtime.
  • UntypedArray since we have a hint from the schema that this is definitely a collection of some kind. We may need to validate runtime vs generation implications but I believe the serializer still returning a value of UntypedArray[UntypeArray[string]] at runtime should be compatible and still work out for us.

@joebalancioLP
Copy link
Author

Have you tried generating other languages (CSharp/Go/Java) to see if you get a different result in the generated code? (not asking to run it for now)

@baywet Thanks for triaging. I haven't tried generating in other languages since TypeScript is my only target for now.

@baywet
Copy link
Member

baywet commented Apr 30, 2024

@andrueastman when you have some time can you investigate to see whether it's specific to TS or general to all languages at this point please?

@andrueastman andrueastman removed TypeScript Pull requests that update Javascript code Needs: Attention 👋 labels May 6, 2024
@andrueastman andrueastman moved this from Todo 📃 to In Progress 🚧 in Kiota May 6, 2024
@andrueastman andrueastman self-assigned this May 6, 2024
@andrueastman
Copy link
Member

Validated this affects other languages as well as the Kiota Builder will recursively unwrap the Items property and still represent the type found as a single dimensional array.
Authored #4606 to address this one.

@andrueastman andrueastman modified the milestones: Backlog, Kiota v1.15 May 6, 2024
@github-project-automation github-project-automation bot moved this from In Progress 🚧 to Done ✔️ in Kiota May 6, 2024
@andrueastman andrueastman added the generator Issues or improvements relater to generation capabilities. label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Issues or improvements relater to generation capabilities. type:bug A broken experience
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants