Skip to content

feat(query-params): add an option to use brackets convention with arrays #428

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

Closed
wants to merge 1 commit into from

Conversation

qboot
Copy link
Contributor

@qboot qboot commented Oct 24, 2022

NB: It's a repost of #420 which (I suppose) has been closed automatically!

Hello 👋

Currently the generated API encode query params arrays like: ?a=foo&a=bar&a=baz with the repeat convention.
But all the backends don't follow this convention, and some use the brackets one: ?a[]=foo&a[]=bar&a[]=baz.

So, this PR adds a new option --query-params-with-brackets which allow the use of the brackets convention instead of the repeat (only applicable when the query param is an array).

@js2me
Copy link
Member

js2me commented Oct 24, 2022

Hello @qboot !
Sorry for closing your PR without any comments.
About your PR - this project is template driven and because of this all things which modifying inside templates is not needed because end user can modify templates as he\she\they want.
One reason of modifying templates - enhancement core template functionality
For example: providing more schema\other information for end user, improve additional jsdoc comments generation, ability to change typescript structs for interfaces\types\enums inside template (current it is not available)

@js2me
Copy link
Member

js2me commented Oct 24, 2022

Also your previous PR was closed because it was target master which exists only for releases
image

image

@qboot
Copy link
Contributor Author

qboot commented Oct 24, 2022

Hello @js2me !

Thank you for your answer. Yes, we have managed to do it with templates in our project, but for me it was missing in the core code of the lib directly, as it's a pretty common thing to have to deal with different query param formats.

And nope, I was targeting the next branch from the master branch of my fork (which was created from the next branch of the lib). Maybe it has confused you.

And what about #421, it was a fix regarding an existant feature of the lib.

Thanks!

@qboot qboot force-pushed the feat/query-params branch from 4e39513 to cd47bd4 Compare December 15, 2022 14:55
@qboot
Copy link
Contributor Author

qboot commented Dec 15, 2022

Hello @js2me

I have finally found the time to revamp this PR. For the record, it allows to use the brackets convention with the fetch API with an option. axios has an option to handle this case, but we need to handle it manually with fetch.

I still find it relevant to do it internally in this lib instead of within my own templates because this case is really standard (it mimics axios + allows the lib to work with a wider variety of backends like Symfony for instance).

Thanks!

@smorimoto smorimoto deleted the branch acacode:next May 10, 2024 12:30
@smorimoto smorimoto closed this May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants