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

feat: migrate to picoquery #337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: migrate to picoquery #337

wants to merge 1 commit into from

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jul 28, 2024

Migrates from qs to picoquery, a much faster library.

We can't use URLSearchParams since we support nested syntax (and therefore assert on the top level property)

@43081j 43081j requested a review from koddsson July 28, 2024 16:00
Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

Is pq drop in? Should we release as major to be safe? Not a suggestion, I’m fine either way just curious.

@43081j
Copy link
Contributor Author

43081j commented Jul 28, 2024

qs has a pretty awkward/messy support for mixed syntax. i think it probably makes sense to bump major because of that

for example, stuff like foo.bar[baz][0] probably works in qs but is nonsense. it should be one syntax or the other, not a mixture (e.g. foo.bar.baz[0], or foo[bar][baz][0] but not both)

we should probably decide which syntax we want to support

picoquery has 3:

  • js (foo.bar[0])
  • dot (foo.bar.0)
  • index (foo[bar][0])

we should probably set it to js i think, though the tests right now assert against the index-style syntax

Migrates from qs to picoquery, a much faster library.
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.

2 participants