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

[rush] Broken peer dependency error when installing @microsoft/rush-lib #2547

Closed
octogonz opened this issue Mar 11, 2021 · 11 comments · Fixed by #2555
Closed

[rush] Broken peer dependency error when installing @microsoft/rush-lib #2547

octogonz opened this issue Mar 11, 2021 · 11 comments · Fixed by #2555
Labels
bug Something isn't working as intended priority The maintainers consider it to be an important issue. We should try to fix it soon.

Comments

@octogonz
Copy link
Collaborator

octogonz commented Mar 11, 2021

Summary

The latest Rush release has a broken peer dependency. This was originally reported by @boardend in #2393 (comment)

Repro steps

$ mkdir repro
$ cd repro
$ npm init
$ npm install @microsoft/rush-lib@5.42.0

Expected result: A clean install.

Actual result:

npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.1.2 (node_modules\chokidar\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.1.3: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN react-native-get-random-values@1.6.0 requires a peer of react-native@>=0.56 but none is installed. You must install peer dependencies yourself.

This is a hard error with --strict-peer-dependencies.

Details

This is a regression in Rush 5.42.0

Standard questions

Question Answer
@microsoft/rush globally installed version? 5.42.0
Operating system? Windows / Mac / Linux
Node.js version (node -v)? 12.20.1
@octogonz octogonz added bug Something isn't working as intended priority The maintainers consider it to be an important issue. We should try to fix it soon. labels Mar 11, 2021
@sachinjoseph
Copy link
Member

Hmmm

PS> md t
PS> cd t
PS> pnpm install @microsoft/rush-lib@5.42.0 --strict-peer-dependencies

image

@sachinjoseph
Copy link
Member

@D4N14L and I did some digging:

image

@D4N14L
Copy link
Member

D4N14L commented Mar 11, 2021

Yes this appears to be an upstream package issue, and has been for some time. I think for now if you don't want this to impact Rush then the AWS cache functionality needs to be removed while the upstream package dependency graph is fixed either to pass the peer dep all the way up to the consumer of @aws-sdk/client-s3, or to satisfy the peer dep.

@octogonz
Copy link
Collaborator Author

react-native is a completely unreasonable dependency. It is 131MB.

Can we at the very least revert the NPM dist tag to Rush 5.41.0 to prevent people from installing 5.42.0?

@octogonz
Copy link
Collaborator Author

octogonz commented Mar 12, 2021

Looks like this awsCredentialsProvider() call is the only part of the feature that relies on the AWS SDK.

try {
credentials = await awsCredentialsProvider()();
} catch {
throw new Error(
"An Amazon S3 credential hasn't been provided, or has expired. " +
`Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", ` +
`or provide an <AccessKeyId>:<SecretAccessKey> pair in the ` +
`${EnvironmentVariableNames.RUSH_BUILD_CACHE_WRITE_CREDENTIAL} environment variable`
);
}

I'm going to make a PR to disable this logic, since it's better than reverting the feature entirely.

Maybe @raihle can help us find a way to perform equivalent functionality without a peer dependency on the entire React Native framework. The peer dependency is not actually needed by this code path BTW. But it is breaking our installs as shown above, so we need to eliminate it.

@octogonz
Copy link
Collaborator Author

Here's a PR: #2550

@octogonz
Copy link
Collaborator Author

This PR did not work because @aws-sdk/client-s3 has the same peer dependency. :'(

@octogonz
Copy link
Collaborator Author

octogonz commented Mar 12, 2021

While we sort this out, I have temporarily rewound the NPM disttag so that latest is 5.41.0.

(So, Amazon... how does a cloud SDK have a dependency on React Native?!)

@octogonz
Copy link
Collaborator Author

We were wondering how this got past our CI build with --strict-peer-dependencies enabled. Turns it the PR included a workaround to delete the dependency locally:

function readPackage(packageJson, context) {
// schema-utils (dependency of webpack-dev-server) has an unfulfilled peer dependency
if (packageJson.name === 'schema-utils') {
if (!packageJson.dependencies) {
packageJson.dependencies = {};
}
packageJson.dependencies['ajv'] = '~6.12.5';
}
if (packageJson.name === '@aws-sdk/middleware-retry') {
delete packageJson.dependencies['react-native-get-random-values'];
}
return packageJson;

@octogonz
Copy link
Collaborator Author

Here is the upstream issue: aws/aws-sdk-js-v3#2051

@raihle
Copy link
Contributor

raihle commented Mar 12, 2021

My bad on this, I expected the pnpmfile hack to disable the dependency for installers, but in retrospect that's of course not possible.

V2 of the AWS SDK is huge by itself and even the earliest versions of V3 include this dependency.

Would a fork of the AWS SDK with this dependency removed be acceptable as a workaround? Or perhaps we can bundle the (required parts of the) AWS SDK while building Rush and keep it as a devDependency?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended priority The maintainers consider it to be an important issue. We should try to fix it soon.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants