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

Stop downloading node_modules when checking yarn peer deps #748

Merged
merged 1 commit into from Nov 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 17 additions & 40 deletions helpers/yarn/lib/peer-dependency-checker.js
Expand Up @@ -15,14 +15,22 @@ const Config = require("@dependabot/yarn-lib/lib/config").default;
const { BufferReporter } = require("@dependabot/yarn-lib/lib/reporters");
const Lockfile = require("@dependabot/yarn-lib/lib/lockfile").default;
const { isString } = require("./helpers");
const fetcher = require("@dependabot/yarn-lib/lib/package-fetcher.js");

// Rely on the package-linker to check for peer dependency requirements
// linker.init(flattenedTopLevelPatterns..) is called from the install command
// which happens after the manifests have been fetched
// Check peer dependencies without downloading node_modules or updating
// package/lockfiles
//
// Logic copied from the import command
class LightweightAdd extends Add {
// Skip updating the lockfile and package.json from the install command
saveLockfileAndIntegrity() {}
persistChanges() {}
async bailout() {
const manifests = await fetcher.fetch(
this.resolver.getManifests(),
this.config
);
this.resolver.updateManifests(manifests);
await this.linker.resolvePeerModules();
return true;
}
}

function devRequirement(requirements) {
Expand Down Expand Up @@ -66,26 +74,18 @@ async function checkPeerDependencies(
directory,
depName,
desiredVersion,
requirements,
topLevelDependencies
requirements
) {
for (let req of requirements) {
await checkPeerDepsForReq(
directory,
depName,
desiredVersion,
req,
topLevelDependencies
);
await checkPeerDepsForReq(directory, depName, desiredVersion, req);
}
}

async function checkPeerDepsForReq(
directory,
depName,
desiredVersion,
requirement,
topLevelDependencies
requirement
) {
const flags = {
ignoreScripts: true,
Expand All @@ -108,29 +108,6 @@ async function checkPeerDepsForReq(
// Returns dep name and version for yarn add, example: ["react@16.6.0"]
let args = installArgsWithVersion(depName, desiredVersion, requirement);

// To check peer dependencies requirements in all top level dependencies we
// need to explicitly tell yarn to fetch all manifests by specifying the
// existing dependency name and version in yarn add

// For exampele, if we have "react@15.6.2" and "react-dom@15.6.2" installed
// and we want to install react@16.6.0, we need get the existing version of
// react-dom and pass this to yarn add along with the new version react, this
// way yarn fetches the manifest for react-dom and determines that we can't
// install react@16.6.0 due to the peer dependency requirement in react-dom

// If we only pass the new dep@version to yarn add, e.g. "react@16.6.0" yarn
// will only fetch the manifest for react and not know that react-dom enforces
// a peerDependency on react

// Returns dep name and version for yarn add, example: ["react-dom@15.6.2"]
// - given react and react-dom in top level deps
const otherDeps = (topLevelDependencies || [])
.filter(dep => dep.name !== depName && dep.version)
.map(dep => installArgsWithVersion(dep.name, dep.version, dep.requirements))
.reduce((acc, dep) => acc.concat(dep), []);

args = args.concat(otherDeps);

// Just as if we'd run `yarn add package@version`, but using our lightweight
// implementation of Add that doesn't actually download and install packages
const add = new LightweightAdd(args, flags, config, reporter, lockfile);
Expand Down
Expand Up @@ -213,8 +213,7 @@ def run_yarn_checker(path:, version:)
Dir.pwd,
dependency.name,
version,
requirements_for_path(dependency.requirements, path),
top_level_dependencies.map(&:to_h)
requirements_for_path(dependency.requirements, path)
]
)
end
Expand Down