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

docs: Update query string package #8871

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

Conversation

nickprinsloo
Copy link
Contributor

The documentation currently suggests using query-string to support more complex structured data with FormData. Specifically it suggests that you can nest data using user[name] syntax.

This form of nesting is explicitly not supported by query-string.

This PR updates the documentation to suggest using qs instead (which does support nesting as described).

The documentation currently suggests using [query-string](https://github.com/sindresorhus/query-string) to support more complex structured data with FormData. Specifically it suggests that you can nest data using `user[name]` syntax. 

This form of nesting is explicitly [not supported](https://github.com/sindresorhus/query-string?tab=readme-ov-file#nesting) by `query-string`. 

This PR updates the documentation to suggest using [qs](https://github.com/ljharb/qs) instead (which does support nesting as described).
Copy link

changeset-bot bot commented Feb 23, 2024

⚠️ No Changeset found

Latest commit: 9dd9820

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@brophdawg11
Copy link
Contributor

I wonder if we should link to both libraries and let folks choose - the qs package is 5x the size of query-string. I know it's used on the server so size is less of a concern. I do think most of the time folks are looking for array support, which query-string has.

@nickprinsloo
Copy link
Contributor Author

FormData natively supports arrays and I'm not sure what using query-string adds? The docs already discuss using FormData in this way for checkboxes but it works for any input. My assumption was that this section was specifically for nested object data which might include but isn't limited to arrays.

@brophdawg11
Copy link
Contributor

FormData natively supports arrays

Agreed - we call that out right above this section in the docs.

I think it's more of the [] notation folks are used to from other languages/frameworks. That was so common in PHP/Rails/etc that we've had a large number of folks think it's built into the spec lol. Most folks don't know that FormData.getAll is a thing and reach for [] out of the box.

My assumption was that this section was specifically for nested object data which might include but isn't limited to arrays

It's for both IMO. The first example works with query-string and qs. The second only with qs. I don't know if we want to default to recommending a 5x larger lib if users only need the first [] example?

<>
  // arrays with []
  <input name="category[]" value="comedy" />
  <input name="category[]" value="comedy" />

  // nested structures parentKey[childKey]
  <input name="user[name]" value="Ryan" />
</>

Maybe we should split that example up and point to the differences in what each lib handles?

If you want [] prefer FormData.getAll, or use query-string or qs. If you want nested objects, use qs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants