-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Replace qs
with zero-dependency & Typesafe neoqs
#65
base: develop
Are you sure you want to change the base?
Conversation
zero-dep, typescript safe alternative to qs
Can also open up a similar pr for lucid as well if y'all would like! Updated: adonisjs/lucid#1048 |
@@ -9,7 +9,8 @@ | |||
|
|||
import raw from 'raw-body' | |||
import inflate from 'inflation' | |||
import qs, { IParseOptions } from 'qs' | |||
import * as qs from 'neoqs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case bodyparser still ships with CJS, this should be neoqs/legacy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with v6 of adonis they migrated entirely to ESM!
Thanks for your work on neoqs and appreciate you getting involved here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!! Thanks a lot for this PR
Heads up, I'm working on neoqs/modern
, which will cut the bundle size from 3.9kb to 3.4kb, by removing all the option invalidation logic, which can be done by typescript entirely. I will then add a PR to switch to that, dropping the size even more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we actually get away with just URLSearchParams or node:querystring ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
depending on the engines that adonis aims to support, I think that's an option as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we actually get away with just URLSearchParams or node:querystring ?
No, we cannot, qs
and neoqs
supports nested object syntax, whereas the node:querystring
module does not.
Thanks for your PR! We discussed this internally, and we would prefer not to migrate to The main reason is that Also, migrating to a brand new package may add more vulnerability since we don't know well the author nor its future intention. If we want to debloat Related PRs are: |
PR inspired by the es tooling ecosystem cleanup initiative on github https://github.com/es-tooling/ecosystem-cleanup
zero-dep, typescript safe alternative to qs
https://deptree.rschristian.dev/?q=qs
https://deptree.rschristian.dev/?q=neoqs
❓ Type of change
📚 Description