Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork753
feat: require an extended body parser#532
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This changes `extended` to be settable to a custom body parser, or`false`.When `false`, the simple parser (`node:querystring`) will be used whichsupports the following syntax:- `foo=a&foo=b` becomes `{"foo":["a","b"]}`- `foo=bar` becomes `{"foo":"bar"}`Otherwise, it must be a function which parses the given string:```ts{ "extended": (bodyStr) => { const params = new URLSearchParams(bodyStr); return convertURLSearchParamsToObject(params); }}```wesleytodd commentedAug 13, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for this, it helps clarify for sure. That said, I am pretty bullish on moving these kind of librariesinto the platform. It is both one of the stated goals of the Node.js Web Server Frameworks Team and our long term vision for express that more of these common requirements are shared and supported via the runtime not each server framework group. So while I agree this is a good general technical decision I hesitate to force end users to go through this change if we are just going to ask them to do another breaking change in the future to move to platform supported extended query string parsing. To me this needs some more discussion so we can have a clear "north star" goal to work toward here. If it happens that this helps get us there, we can totally merge it, but I want to know what that end goal looks like before making decisions here. |
43081j commentedAug 13, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If your hope is that node ships a query string parser capable of nesting, it is very unlikely it would have the behaviour qs has (since it's such a mixed bag of behaviours without any standard) So I think even if you keep this as is, and node ships that one day, you'll be asking users to change the behaviour of their apps. The only way you would avoid that is by having a qs-like layer around node's parser (if it existed) If you do that, you're then constrained by a very loosely defined third party API (qs) rather than shaping around what is in the platform |
benmccann commentedJun 24, 2025
I just ran into a need for this. I'd love to see it move forward. I agree with@43081j that I don't see a path for Node to adopt the logic that lives in |
Uh oh!
There was an error while loading.Please reload this page.
This changes
extendedto be settable to a custom body parser, orfalse.When
false, the simple parser (node:querystring) will be used which supports the following syntax:foo=a&foo=bbecomes{"foo":["a","b"]}foo=barbecomes{"foo":"bar"}Otherwise, it must be a function which parses the given string:
This way the library no longer enforces a specific query string parser, but instead pushes the user to bring their own.
Most users will be fine with
URLSearchParams, so it may make sense for us to implement a very basic conversion of that to a plain object (like in the example above).If anyone wants more than that, they can bring their own library and pass it in.
Notes:
extended. Rather we would introduce something else I think, likebodyParserURLSearchParamsto object parse function which becomes the default, or just usenode:querystring(no object nesting)cc@wesleytodd this is what i was trying to explain in my comments elsewhere in the other PR. i don't think
body-parsershould be shipping with a query string parser since it may not always be enabled (so would be a waste). I think we should have a very basic nested parser or none at alli did this to explain to you what was in my head. if its the totally opposite direction of whats in yours, please feel free to discard the PR. i'll leave it as draft meanwhile