Skip to content

chore: reduce dependencies from qs #2407

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

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

Conversation

benmccann
Copy link

@benmccann benmccann commented Jun 2, 2025

https://npmgraph.js.org/?q=qs - 18 dependencies
https://npmgraph.js.org/?q=neoqs - 0 dependencies

I used the legacy entry point to keep CJS support

Checklist

  • Sign off your commits with DCO (Developer Certificate of Origin)
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@benmccann benmccann requested a review from dhmlau as a code owner June 2, 2025 15:25
@dhmlau
Copy link
Member

dhmlau commented Jun 4, 2025

@benmccann, thanks for the PR. While I understand the reason to switch to neoqs, I'm concerned about the maturity and the download volume of neoqs comparing with qs. I'd prefer to keeping qs.
@loopbackio/loopback-maintainers, any thoughts?

@benmccann
Copy link
Author

There are several other alternatives that could be chosen from: https://github.com/es-tooling/module-replacements/blob/fa9d67b02c9feb28c75bd3509930234b6f5383b1/docs/modules/qs.md

I chose neoqs because you already use neotraverse by the same author: 5e04826

@dhmlau
Copy link
Member

dhmlau commented Jun 4, 2025

@benmccann, thanks for pointing it out. As you might've noticed already, neotraverse has a much higher download numbers.
Do you have particular concerns of using qs besides the fact that qs has dependencies while neoqs has none?

@benmccann
Copy link
Author

Yeah, you're right that neotraverse has much higher download numbers than neoqs. I wasn't sure how much that mattered since you'd made the switch from traverse to neotraverse eventhough traverse has 10x as many downloads.

I guess you're thinking more downloads means it's more battle tested? Though neoqs shares a test suite with qs, so I think they have basically the same level of robustness and the same history of being battle tested. The reason I pointed out that it was the same author as neotraverse is to share that it's a very reputable author. I've interacted with the authors of both libraries many times via other projects and have generally seen the neotraverse author to be far more responsive when there's any issue.

As an alternative, https://www.npmjs.com/package/qs-esm is more downloaded, but it looks like this project still supports CJS and Node 18 and you can't require an ESM library until Node 20. I see that Node 18 was just dropped in core in loopbackio/loopback-next@88a3fe9, so perhaps we could bump the Node version requirement here as well and use that library?

Yeah, my issue is how many dependencies are required. It's easy to think of it as not being that big of an issue, but the problem really adds up when using many dependencies. As an example, just installing the core docusaurus library without any plugins or anything adds up to over 1000 dependencies (https://npmgraph.js.org/?q=@docusaurus/core). This makes debugging anything a nightmare because you have to go through so many layers of libraries and there's so much code in the distributable not to mention the supply chain risks. As a result, it's something I check with every new library I adopt.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 15396214664

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.711%

Totals Coverage Status
Change from base Build 15382976313: 0.0%
Covered Lines: 7278
Relevant Lines: 8284

💛 - Coveralls

@dhmlau
Copy link
Member

dhmlau commented Jun 5, 2025

I'm good with either way. Would like to hear from other maintainers, especially @samarpanB @achrinza @raymondfeng.

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.

3 participants