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

Add support for array query parameters #52

Merged
merged 1 commit into from
Feb 16, 2022
Merged

Add support for array query parameters #52

merged 1 commit into from
Feb 16, 2022

Conversation

rapkis
Copy link

@rapkis rapkis commented Feb 3, 2022

I've added a feature which allows to generate URLs with array query parameters. URLs often have such parameters, so I think this feature is useful for this package.

The changes may be a bit controversial so I'm open for feedback. I think that using native PHP functions is better, since the implementation problem is already solved in them, and there's no longer a need for the Arr helper class.
I'm not sure why these functions weren't used previously, so if there was a reason - I'd appreciate some feedback here.
The only downside I see with parse_str is this (details):

variables in PHP can't have dots and spaces in their names, those are converted to underscores

Main changes:

  • use native http_build_query function to cast QueryParameterBag to string
  • use native parse_str function to parse input in QueryParameterBag fromString() method
  • remove helper class as it is no longer used
  • use union types for the set() method, in order to clearly support string and array only
  • fix unit tests to cover new feature

Whatever implementation fits best - I'm mostly interested in the array support itself. If there's any questions regarding it - please let me know.

- use native `http_build_query` function to cast QueryParameterBag to string
- use native `parse_str` function to parse string input for QueryParameterBag `fromString()`
- remove helper class as it is no longer used
- fix unit tests to cover new feature
@AidasK
Copy link

AidasK commented Feb 15, 2022

Can we merge this? Looks like a good patch

@freekmurze freekmurze merged commit 08b0f20 into spatie:main Feb 16, 2022
@freekmurze
Copy link
Member

Thanks!

@rapkis rapkis deleted the feature/support-array-query branch February 18, 2022 20:23
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