-
-
Notifications
You must be signed in to change notification settings - Fork 28
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 neoqs #88
base: 7.x
Are you sure you want to change the base?
Conversation
Looks nice. But, I think I will inspect the |
It is super new. It is part of an initiative to debloat the Node ecosystem by cleaning packages from "we know who". 😄 I was planning to rewrite |
Yeah, that's why want to run a few tests myself. Since, the query string parsing it pretty much used by every AdonisJS app running in prod. I want to make sure we do not break anything (even by mistake) :) |
Hi @thetutlage! Creator of neoqs here. If you have any questions you'd like to ask me, feel free to do so, and I'll do my best to answer them! |
@@ -7,7 +7,7 @@ | |||
* file that was distributed with this source code. | |||
*/ | |||
|
|||
import { parse, stringify } from 'qs' | |||
import { parse, stringify } 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.
If this package still ships CommonJS in any capacity, this should be neoqs/legacy
. Same for the other file
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: |
Thanks a lot for the response! As the author of the package, I'd like to address some of your concerns:
I am tracking qs for every release. Whenever qs cuts a release, I'll backport the same code to neoqs, so it'll always be up to date(barring a few days to a week based on my bandwidth). Same for all the tests, neoqs won't cut a release if all the tests don't pass the qs tests.
This is understandable. However I'd like to point out that qs has 14 transitive dependency, and any one of them going rogue/being compromised poses a larger risk, compared to As for the package's intentions, it is to stay 0-dependency forever, and modernise overtime to cut down the bloat.
I respect that! Feel free to copy https://github.com/PuruVJ/neoqs/tree/main/src to ur own package with proper attribution. Have a good day |
Hello @PuruVJ Thanks for taking the feedback positively. One quick question. Do you plan to maintain the feature set of |
Thanks for the quick reply! I won't remove any features of my own accord. If qs adds a feature, I will add it. if qs removes a feature, I will remove it as well. |
@PuruVJ Good to know :) |
Hey there! 👋🏻
This PR replaces the usage of
qs
forneoqs
.neoqs
is a drop-in replacement forqs
without the bloat.Moving to it means:
QS Dependencies
NeoQS Dependencies