Skip to content

feat(core): add better support for form-data arrays #2026

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

Merged
merged 6 commits into from
Apr 17, 2025

Conversation

AllieJonsson
Copy link
Contributor

Status

READY

Description

Fixes #1417 (appending [] to formData key name) and adds support for exploding the formData properties as well.
I need this in my current project at work, so I added it here as well.

melloware
melloware previously approved these changes Apr 14, 2025
@melloware melloware requested a review from soartec-lab April 14, 2025 15:07
@AllieJonsson AllieJonsson changed the title Feat/form data array feat(core): add union enum generation Apr 14, 2025
@AllieJonsson AllieJonsson changed the title feat(core): add union enum generation feat(core): add better support for form-data arrays Apr 14, 2025
melloware
melloware previously approved these changes Apr 14, 2025
@soartec-lab
Copy link
Member

I'll check it later!

@soartec-lab soartec-lab self-assigned this Apr 15, 2025
@anymaniax
Copy link
Collaborator

@AllieJonsson not related to the pull request. Could you check your discord messages when you have the time

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.

@AllieJonsson
I added comments 👍
could you check it ?

}
```

Type `explode` setting results in the following generated code:
Copy link
Member

Choose a reason for hiding this comment

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

This may not be necessary.
If necessary, why not consider specifying explode separately?

https://swagger.io/docs/specification/v3_0/serialization/

Copy link
Member

Choose a reason for hiding this comment

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

And if this is removed, it can be made into a simple option like override.formData.serializeWithBrackets: true 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that doesn't allow you to specify how body data in multipart/form-data requests are serialized, right?

Copy link
Member

Choose a reason for hiding this comment

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

I thought that if override.formData.serializeWithBrackets was set to true, brackets would be added, and if it was set to false, it would behave as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was thinking about the explode, how would you configure it to explode form data?
What I need for it to work with my ASP.Net backend is instead of

data.forEach(
  (value, index) => {
    formData.append(
      `receptionFacilitiesPerFaction`,
/* 
 * value is of type
 * {
 *   fractionId: number
 *   receptionFacilityIds: number[]
 * }
*/
      JSON.serialize(value),
    );
  },
);

I need to set

data.forEach(
  (value, index) => {
    formData.append(
      `receptionFacilitiesPerFaction[${index}].fractionId`,
      value.fractionId.toString(),
    );
    value.receptionFacilityIds.forEach((v, rIndex) => {
      formData.append(
        `receptionFacilitiesPerFaction[${index}].receptionFacilityIds[${rIndex}]`,
        v.toString(),
      );
    });
  },
);

Copy link
Member

Choose a reason for hiding this comment

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

Wow, so ASP.net can't accept it unless it's in a format like receptionFacilitiesPerFaction[0]&receptionFacilitiesPerFaction[1]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was surprised as well, I couldn't get it to work with sending json objects in a multipart/form-data, so I asked ChatGPT, which gave me this solution, and it worked!
https://chatgpt.com/share/67ff8e02-1fb0-8000-901b-61e336ddad0e

Copy link
Member

Choose a reason for hiding this comment

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

I see, I didn't know that.

Copy link
Member

Choose a reason for hiding this comment

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

@AllieJonsson
Okey, then we already have override.formData so let's just move up the options level like override.formData.arrayHandling.

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.

It's good!

@soartec-lab
Copy link
Member

@AllieJonsson
I merged #2038 so could you try rebase master?

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!

@soartec-lab soartec-lab merged commit 58ce907 into orval-labs:master Apr 17, 2025
2 checks passed
@soartec-lab soartec-lab added this to the 7.9.0 milestone Apr 20, 2025
@soartec-lab soartec-lab added the enhancement New feature or request label Apr 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

form-data array submission support
4 participants