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

fix(fetch): properly append array values to normalizedParams object #1580

Closed
wants to merge 1 commit into from

Conversation

n2k3
Copy link

@n2k3 n2k3 commented Aug 16, 2024

Status

WIP

Description

related #1285

I experienced a problem with array values.
For example I want to send this: Foo: [0, 1] this will be added by this line of code to the URLSearchParams instance, which in turn will by stringified to Foo=0,1 and appended to the API url in generated code.

Shouldn't this be done according to this example in the MDN docs? Resulting in Foo=0&Foo=1 as stringified value.

This PR addresses that. Let me know if I need to anything else within this PR.

Thanks @soartec-lab for the great work on the fetch client 🎉

Related PRs

List related PRs against other branches:

Todos

  • Tests
  • Documentation
  • Changelog Entry (unreleased)

@melloware melloware added the fetch Fetch client related issue label Aug 16, 2024
@melloware melloware added this to the 7.0.2 milestone Aug 16, 2024
@melloware
Copy link
Collaborator

Looks like the build is failing?

@melloware melloware requested a review from soartec-lab August 16, 2024 12:14
@n2k3
Copy link
Author

n2k3 commented Aug 19, 2024

Looks like the build is failing?

I can now reproduce locally, I'm working on a fix 👍

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I'm wondering if I should make it optional and selectable. Because this is currently working.

@n2k3
Copy link
Author

n2k3 commented Aug 22, 2024

Thanks! I'm wondering if I should make it optional and selectable. Because this is currently working.

I've found the cause of the error, it's because the strong typed pet store example /samples/**/models/listPetsParams.ts doesn't contain any parameters that have an array type. In the generated example it only is

export type ListPetsParams = {
  /**
   * How many items to return at one time (max 100)
   */
  limit?: string;
};

this is based on the /samples/**/petstore.yaml

      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: string

The error claims that the code value instanceof Array is never true, because it is inferred that value can only be of type string not of any kind of Array

So then the proposed change of the PR would have to be dynamically included based on whether the type is an array or not. I haven't figured out how / where to do this best. If you have any idea on how to do this properly, don't hesitate to do so 😉

@soartec-lab
Copy link
Member

This means changing the conditions for constructing query parameters depending on the type of OpenAPI, right?
I will also look for ways to change it 👍

@soartec-lab
Copy link
Member

Hi, @n2k3
I looked into this.
A reasonable way to accomplish this is to parse the generated query params string and verify whether it contains an array. Also, consider cases where there are only array-type fields.

@soartec-lab
Copy link
Member

@n2k3
I created #1604 based on this PR. Does control with explode property solve your problem?

@soartec-lab
Copy link
Member

I'll close this PR. Because, this issue fixed by #1604.

@soartec-lab soartec-lab closed this Sep 1, 2024
@n2k3 n2k3 deleted the fetch-array-values branch September 17, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetch Fetch client related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants