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

fix(dependencies): replace qs by neoqs #6291

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

fix(dependencies): replace qs by neoqs #6291

wants to merge 1 commit into from

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Jul 23, 2024

Summary

Replace qs by neoqs. This doesn't have a bundlesize change, but it avoids us ever accidentally updating qs and having a big bump

Result

neoqs is API-compatible with qs in the legacy mode, and thus this isn't a breaking change. Of course if someone was already using qs and relying on it being deduplicated with the InstantSearch version.

URLs parsed and created by qs and neoqs are compatible

neoqs is API-compatible with qs in the legacy mode, and thus this isn't a breaking change. Of course if someone was already using qs and relying on it being deduplicated with the InstantSearch version.

URLs parsed and created by qs and neoqs are compatible
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9f92e8c:

Sandbox Source
example-instantsearch-getting-started Configuration
example-react-instantsearch-getting-started Configuration
example-react-instantsearch-next-app-dir-example Configuration
example-react-instantsearch-next-routing-example Configuration
example-vue-instantsearch-getting-started Configuration

@Haroenv
Copy link
Contributor Author

Haroenv commented Jul 23, 2024

Hmm bundle size is larger, doesn't seem to be due to non-working deduplication, maybe other features added since qs@6.9.7? cc @puruvjdev is this expected?

@@ -11,7 +11,7 @@
"algoliasearch": "4.23.2",
"core-js": "2",
"instantsearch.css": "8.3.0",
"qs": "6.9.7",
"neoqs": "6.2.12",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 6.12.3, I released slightly wrong version.

@PuruVJ
Copy link

PuruVJ commented Jul 23, 2024

Hmm bundle size is larger, doesn't seem to be due to non-working deduplication, maybe other features added since qs@6.9.7? cc @puruvjdev is this expected?

I am looking at bundlephobia to see if that's the case, but it seems to be down. I'll let you know if that's the case.

EDIT: well, its after 6.9.7 only that the library went crazy

CleanShot 2024-07-23 at 19 22 54

@PuruVJ
Copy link

PuruVJ commented Jul 23, 2024

Well the real thing is that /legacy is compiled to ES5, so it does add an extra 400bytes(min+br). That is the additional size we're seeing here. This is done so even very old libs and apps can use neoqs and avoid dependency hell. Also, there have been some additional features since 6.9.7 which do account for a few extra bytes here.

Only way the size would go down is by using the default build, which is ESM-only.

I'd suggest using neoqs/legacy now with the extra 1KB, if it's not too much, but whenever in future you decide to drop CJS support, switch to neoqs/modern(It's gonna be 3.4KB only 👀, releasing sometime next week).

Lemme know if that helps.
And thanks for considering this lib

@PuruVJ
Copy link

PuruVJ commented Aug 2, 2024

Any update on this? 😁

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.

2 participants