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: switch to qs and add tests for query string #56

Merged
merged 1 commit into from
Apr 23, 2023

Conversation

mnahkies
Copy link
Owner

  • switch to qs package as this advertises support for arrays/objects more so than the built-in node:querystring library

  • add tests for query string serialization

I'm not 100% sure that this is actually what we want, or at least what we always want. It could well be that we need to provide configuration to control how arrays/objects are handled for serialization, as I suspect different API's will have different opinions on this.

Currently it's also ignoring the serialization hint parameters like style/explode - just adding a todo for now.

- switch to qs package as this advertises support for arrays/objects
  more so than the built-in node:querystring library

- add tests for query string serialization

I'm not 100% sure that this is actually what we want, or at least what
we always want. It could well be that we need to provide configuration
to control how arrays/objects are handled for serialization, as I
suspect different API's will have different opinions on this.

Currently it's also ignoring the serialization hint parameters like
style/explode - just adding a todo for now.
@mnahkies mnahkies enabled auto-merge (squash) April 23, 2023 06:45
@mnahkies mnahkies merged commit dbcec86 into master Apr 23, 2023
2 checks passed
@mnahkies mnahkies deleted the mn/fix/query-strings branch April 23, 2023 06:48
mnahkies added a commit that referenced this pull request Apr 23, 2023
- obtained from https://github.com/stripe/openapi

- licensed as MIT
(https://github.com/stripe/openapi/blob/master/LICENSE)
  although this is currently missing from the specification itself
  (see stripe/openapi#120)

surfaced numerous issues that are now fixed:
- #50
- #55
- #56
- #57
- #58
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.

None yet

1 participant