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

middleware-retry includes react-native #2051

Closed
kyeotic opened this issue Feb 19, 2021 · 12 comments · Fixed by #2191
Closed

middleware-retry includes react-native #2051

kyeotic opened this issue Feb 19, 2021 · 12 comments · Fixed by #2191
Assignees
Labels
bug This issue is a bug.

Comments

@kyeotic
Copy link

kyeotic commented Feb 19, 2021

Describe the bug

This otherwise innocuous package includes react-native-get-random-values, which in turn installs all of react-native. It's listed as a production dependency, though it does not appear to be used.

Its adds over 100mb to the install size of the dynamodb-client

❯ npm ls react-native
[email protected] /Users/kyeotic/dev/dynamo-butter
└─┬ @aws-sdk/[email protected]
  └─┬ @aws-sdk/[email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └─┬ @react-native-community/[email protected]
          └── [email protected] deduped

I understand that this is done to handle this issue while running in certain environments, but this is a nuclear solution. I think it would be better to push this onto apps that need it, or find a uuid package that doesn't require all of react-native to be installed.

For users in node environments this is a huge counter to the modular nature of this sdk.

@kyeotic kyeotic added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 19, 2021
@StefanFeederle
Copy link

StefanFeederle commented Feb 20, 2021

Can confirm. Somehow this is only an issue on node v15. On v14 react-native and it's dependencies are not installed.

Found this issue while upgrading my docker build setup from node 14 to 15.

10 biggest folders in node_modules with node v14:
36716 node_modules/sharp
35556 node_modules/sharp/vendor
11336 node_modules/@AWS-SDK
7460 node_modules/core-js
5156 node_modules/moment
5120 node_modules/@aws-sdk/client-s3
4976 node_modules/lodash
3616 node_modules/object.values
3616 node_modules/object.getownpropertydescriptors
3584 node_modules/object.values/node_modules

10 biggest folders in node_modules with node v15:
39216 node_modules/jsc-android
39208 node_modules/jsc-android/dist
38768 node_modules/react-native
36712 node_modules/sharp
35556 node_modules/sharp/vendor
18136 node_modules/hermes-engine
13428 node_modules/react-native/android
11376 node_modules/hermes-engine/android
11332 node_modules/@AWS-SDK
9940 node_modules/react-native/Libraries

This added at least 7 huge dependencies!
Now only 3 of my direct dependencies show up in the top10!

Edit: This happens due to npm v7 switching to installing all peerDependencies by default!
Es a workaround you can use a npm-flag:
npm install --legacy-peer-deps

@kyeotic
Copy link
Author

kyeotic commented Feb 21, 2021

This happens to me on Node 12 and Node 14

@StefanFeederle
Copy link

What version of npm are you using?

@kyeotic
Copy link
Author

kyeotic commented Feb 21, 2021

Im using npm 7, which is the "latest" version at time of writing

@StefanFeederle
Copy link

This happens due to npm v7 switching to installing all peerDependencies by default
As a workaround you can use a npm-flag:
npm install --legacy-peer-deps

@patchu
Copy link

patchu commented Feb 25, 2021

+1 for replacing this library. Deep inside react-native is a DOS issue with node-fetch, and it's showing up as a low-severity vulnerability (actually, showing up as 10 vulnerabilities). "npm audit" says there's no fix available.

At this point, I have to inform all downstream users of my library to use --legacy-peer-deps, and then install peer dependencies manually.

@lucasff
Copy link

lucasff commented Mar 2, 2021

Also can reproduce the problem, using Yarn.
@aws-amplify/auth > @aws-amplify/core > @aws-sdk/client-cognito-identity > @aws-sdk/middleware-retry > [email protected]"

I'm trying to import only the Auth portion of the lib.

coderbyheart added a commit to NordicSemiconductor/cloud-aws-package-layered-lambdas-js that referenced this issue Mar 3, 2021
coderbyheart added a commit to NordicSemiconductor/asset-tracker-cloud-aws-js that referenced this issue Mar 3, 2021
coderbyheart added a commit to NordicSemiconductor/asset-tracker-cloud-aws-js that referenced this issue Mar 3, 2021
@bfricka
Copy link

bfricka commented Mar 3, 2021

There are plenty of ways of getting crypto random values. Anyways, this is in dependencies b/c index.native.js imports it for side effects (adding getRandomValues to global.crypto). There's got to be a better way to do this. In my case, I'm using Yarn v2, and only using the node implementation, so I'm shutting it off in .yarnrc.yml:

packageExtensions:
  "@aws-sdk/middleware-retry@*":
    peerDependenciesMeta:
      react-native:
        optional: true

coderbyheart added a commit to NordicSemiconductor/asset-tracker-cloud-aws-js that referenced this issue Mar 5, 2021
@G-Rath
Copy link

G-Rath commented Mar 5, 2021

This is related to #1536

@octogonz
Copy link

octogonz commented Mar 12, 2021

We got burned by this issue today in microsoft/rushstack#2547

This happens due to npm v7 switching to installing all peerDependencies by default

NPM's decision to automatically install missing peer dependencies was controversial. (See this position statement from the Yarn maintainer: npm/rfcs#43 (comment) .) In strict mode, other package managers simply report an error if react-native is not included, because neglecting to install a peer dependency produces a technically incorrect installation.

Even if we install react-native (or allow NPM to automagically install it), that package brings in 131 MB of mobile device development framework that has nothing to do with AWS. It's not a reasonable dependency.

@octogonz
Copy link

Update: Rush worked around this by entirely eliminating the dependency on AWS SDK. The relevant REST call turned out to be simple enough to hand code (microsoft/rushstack#2555).

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants