amplitude / Amplitude-JavaScript

JavaScript SDK for Amplitude
MIT License
314 stars 132 forks source link

Why is query-string fixed to v5? This cause multiple query string in my final webapp bundle #548

Open kopax-polyconseil opened 2 years ago

kopax-polyconseil commented 2 years ago

Expected Behavior

I expect one query-string module in my final web app bundle

Current Behavior

strict-uri-encode (Found 2 resolved, 2 installed, 2 depended. Latest 2.0.0.) 1.1.0 ~/strict-uri-encode PassCulture@1.199.0 -> amplitude-js@^8.16.1 -> query-string@5 -> strict-uri-encode@^1.0.0 2.0.0 ~/@react-navigation/core/~/strict-uri-encode PassCulture@1.199.0 -> @react-navigation/native@^6.0.6 -> @react-navigation/core@^6.1.0 -> query-string@^7.0.0 -> strict-uri-encode@^2.0.0

Possible Solution

Do not fix query string version and upgrade it to latest

Steps to Reproduce

Not necessary

Environment

yuhao900914 commented 2 years ago

Hi @kopax-polyconseil Thanks for using our SDKs. Using the @latest version for one package might also have the same issue because other packages which have the same dependency might not use the same criteria. You can use the npm dedupe to reduce the duplication in the package tree. https://docs.npmjs.com/cli/v8/commands/npm-dedupe. Hope this works.

kopax-polyconseil commented 2 years ago

Thanks for your reply, problem here is that those package are not maintained anymore, perhaps an upgrade of the deps on all the dep tree would be the right move ? There's a reason why new versions come out, I hope you know it.

kopax-polyconseil commented 2 years ago

@yuhao900914 also we use yarn, and

dka@dka:[~/workspace/github.com/pass-culture/pass-culture-app-native (PC-16624)]: yarn dedupe query-string
yarn dedupe v1.22.15
error The dedupe command isn't necessary. `yarn install` will already dedupe.
info Visit https://yarnpkg.com/en/docs/cli/dedupe for documentation about this command.
dka@dka:[~/workspace/github.com/pass-culture/pass-culture-app-native (PC-16624)]: ^C
dka@dka:[~/workspace/github.com/pass-culture/pass-culture-app-native (PC-16624)]: yarn dedupe --strategy highest
yarn dedupe v1.22.15
error The dedupe command isn't necessary. `yarn install` will already dedupe.
info Visit https://yarnpkg.com/en/docs/cli/dedupe for documentation about this command.
dka@dka:[~/workspace/github.com/pass-culture/pass-culture-app-native (PC-16624)]: yarn dedupe --check
yarn dedupe v1.22.15
error The dedupe command isn't necessary. `yarn install` will already dedupe.
info Visit https://yarnpkg.com/en/docs/cli/dedupe for documentation about this command.
yuhao900914 commented 2 years ago

Hi @kopax-polyconseil yarn install --flat is the equivalent to npm dedupe. yarn install does not dedupe the package very well. Another workaround is to add a resolution in your package.json. { "name": "amplitude-js-demo", "version": "0.1.0", "private": true, "resolutions": { "query-string": "^7.1.1" }, xxxx }

The query-string@5 to the latest version has breaking changes. To support the older browser, we need to use query-string@5.

kopax-polyconseil commented 2 years ago

Thanks for your reply, on a big project like ours yarn install --flat is a bit risky, I've had a try and it's almost all our dependency that will move. I am not willing to take this risk, it will touch our react native and also the web application, while I only want to optimize the web build. But thanks for offering.

I manage to solve the dedupe of this problem using webpack resolve alias, and I will have another try using the resolution :)

k-mckinney commented 1 year ago

Snyk jobs are now failing in our pipelines because of a downstream DoS vulnerability coming from a dependency in query-string:

amplitude-js@8.18.1 > query-string@5.1.1 > decode-uri-component@0.2.0

This has been fixed in v0.2.2 of decode-uri-component, and that's used in v7.1.3 of query-string, but if this project is pinned to v5 of query-string that vulnerability will continue to exist.

guncebektas commented 1 year ago

same issue at 8.21.4, setting "amplitude-js": "8.21.3" in package.json removes the error