fix(core): prefer project specific external deps

packages/nx/src/plugins/js/project-graph/build-dependencies/build-dependencies.ts

@@ -3,6 +3,19 @@ import { buildExplicitPackageJsonDependencies } from './explicit-package-json-de
 import { CreateDependenciesContext } from '../../../../project-graph/plugins';
 import { RawProjectGraphDependency } from '../../../../project-graph/project-graph-builder';
 
+/**
+ * When processing external dependencies, we need to keep track of which version of that dependency
+ * is most appropriate for each source project. A workspace may contain multiple copies of a dependency
+ * but we should base the graph on the version that is most relevant to the source project, otherwise
+ * things like the dependency-check lint rule will not work as expected.
+ *
+ * We need to track this in this separate cache because the project file map that lives on the ctx will
+ * only be updated at the very end.
+ *
+ * sourceProject => resolved external node names (e.g. 'npm:[email protected]')
+ */
+export type ExternalDependenciesCache = Map<string, Set<string>>;
+
 export function buildExplicitDependencies(
  jsPluginConfig: {
  analyzeSourceFiles?: boolean;
@@ -12,8 +25,23 @@ export function buildExplicitDependencies(
 ): RawProjectGraphDependency[] {
  if (totalNumberOfFilesToProcess(ctx) === 0) return [];
 
+ const externalDependenciesCache: ExternalDependenciesCache = new Map();
  let dependencies: RawProjectGraphDependency[] = [];
 
+ /**
+ * NOTE: It is important that we process package.json files first so that accurate versions for
+ * external dependencies are captured in the ExternalDependenciesCache before converting
+ * imports within TS files.
+ */
+ if (
+ jsPluginConfig.analyzePackageJson === undefined ||
+ jsPluginConfig.analyzePackageJson === true
+ ) {
+ dependencies = dependencies.concat(
+ buildExplicitPackageJsonDependencies(ctx, externalDependenciesCache)
+ );
+ }
+
  if (
  jsPluginConfig.analyzeSourceFiles === undefined ||
  jsPluginConfig.analyzeSourceFiles === true
@@ -25,18 +53,10 @@ export function buildExplicitDependencies(
  } catch {}
  if (tsExists) {
  dependencies = dependencies.concat(
- buildExplicitTypeScriptDependencies(ctx)
+ buildExplicitTypeScriptDependencies(ctx, externalDependenciesCache)
  );
  }
  }
- if (
- jsPluginConfig.analyzePackageJson === undefined ||
- jsPluginConfig.analyzePackageJson === true
- ) {
- dependencies = dependencies.concat(
- buildExplicitPackageJsonDependencies(ctx)
- );
- }
 
  return dependencies;
 }

packages/nx/src/plugins/js/project-graph/build-dependencies/explicit-package-json-dependencies.spec.ts

@@ -105,28 +105,35 @@ describe('explicit package json dependencies', () => {
  tempFs.cleanup();
  });
 
- it(`should add dependencies for projects based on deps in package.json`, () => {
- const res = buildExplicitPackageJsonDependencies(ctx);
-
+ it(`should add dependencies for projects based on deps in package.json and populate the cache`, () => {
+ const cache = new Map();
+ const res = buildExplicitPackageJsonDependencies(ctx, cache);
  expect(res).toEqual([
  {
  source: 'proj',
- target: 'proj2',
+ target: 'proj3',
  sourceFile: 'libs/proj/package.json',
  type: 'static',
  },
  {
- sourceFile: 'libs/proj/package.json',
  source: 'proj',
- target: 'npm:external',
+ target: 'proj2',
+ sourceFile: 'libs/proj/package.json',
  type: 'static',
  },
  {
- source: 'proj',
- target: 'proj3',
  sourceFile: 'libs/proj/package.json',
+ source: 'proj',
+ target: 'npm:external',
  type: 'static',
  },
  ]);
+ expect(cache).toMatchInlineSnapshot(`
+ Map {
+ "proj" => Set {
+ "npm:external",
+ },
+ }
+ `);
  });
 });

packages/nx/src/plugins/js/project-graph/build-dependencies/explicit-package-json-dependencies.ts

@@ -1,22 +1,24 @@
-import { defaultFileRead } from '../../../../project-graph/file-utils';
 import { join } from 'path';
 import { DependencyType } from '../../../../config/project-graph';
-import { parseJson } from '../../../../utils/json';
-import { joinPathFragments } from '../../../../utils/path';
 import {
  ProjectConfiguration,
  ProjectsConfigurations,
 } from '../../../../config/workspace-json-project-json';
-import { NxJsonConfiguration } from '../../../../config/nx-json';
-import { PackageJson } from '../../../../utils/package-json';
+import { defaultFileRead } from '../../../../project-graph/file-utils';
 import { CreateDependenciesContext } from '../../../../project-graph/plugins';
 import {
  RawProjectGraphDependency,
  validateDependency,
 } from '../../../../project-graph/project-graph-builder';
+import { parseJson } from '../../../../utils/json';
+import { PackageJson } from '../../../../utils/package-json';
+import { joinPathFragments } from '../../../../utils/path';
+import { ExternalDependenciesCache } from './build-dependencies';
+import { satisfies } from 'semver';
 
 export function buildExplicitPackageJsonDependencies(
- ctx: CreateDependenciesContext
+ ctx: CreateDependenciesContext,
+ externalDependenciesCache: ExternalDependenciesCache
 ): RawProjectGraphDependency[] {
  const res: RawProjectGraphDependency[] = [];
  let packageNameMap = undefined;
@@ -26,7 +28,14 @@ export function buildExplicitPackageJsonDependencies(
  if (isPackageJsonAtProjectRoot(nodes, f.file)) {
  // we only create the package name map once and only if a package.json file changes
  packageNameMap = packageNameMap || createPackageNameMap(ctx.projects);
- processPackageJson(source, f.file, ctx, res, packageNameMap);
+ processPackageJson(
+ source,
+ f.file,
+ ctx,
+ externalDependenciesCache,
+ res,
+ packageNameMap
+ );
  }
  });
  });
@@ -63,13 +72,43 @@ function processPackageJson(
  sourceProject: string,
  fileName: string,
  ctx: CreateDependenciesContext,
+ externalDependenciesCache: ExternalDependenciesCache,
  collectedDeps: RawProjectGraphDependency[],
  packageNameMap: { [packageName: string]: string }
 ) {
  try {
  const deps = readDeps(parseJson(defaultFileRead(fileName)));
+
+ function applyDependencyTarget(target: string) {
+ const dependency: RawProjectGraphDependency = {
+ source: sourceProject,
+ target: target,
+ sourceFile: fileName,
+ type: DependencyType.static,
+ };
+ validateDependency(dependency, ctx);
+ collectedDeps.push(dependency);
+ const depsForSource =
+ externalDependenciesCache.get(sourceProject) || new Set();
+ depsForSource.add(dependency.target);
+ externalDependenciesCache.set(sourceProject, depsForSource);
+ }
+
+ // Preprocess relevant external nodes for the given deps for optimal traversal
+ const depToRelevantExternalNodes = new Map();
+ for (const externalNode of Object.keys(ctx.externalNodes)) {
+ const [depName] = externalNode.match(/^npm:(.*)/)?.[1].split('@') ?? [];
+ if (!deps[depName]) {
+ continue;
+ }
+ if (!depToRelevantExternalNodes.has(depName)) {
+ depToRelevantExternalNodes.set(depName, []);
+ }
+ depToRelevantExternalNodes.get(depName).push(externalNode);
+ }
+
  // the name matches the import path
- deps.forEach((d) => {
+ Object.entries(deps).forEach(([d, version]) => {
  // package.json refers to another project in the monorepo
  if (packageNameMap[d]) {
  const dependency: RawProjectGraphDependency = {
@@ -80,15 +119,42 @@ function processPackageJson(
  };
  validateDependency(dependency, ctx);
  collectedDeps.push(dependency);
- } else if (ctx.externalNodes[`npm:${d}`]) {
- const dependency: RawProjectGraphDependency = {
- source: sourceProject,
- target: `npm:${d}`,
- sourceFile: fileName,
- type: DependencyType.static,
- };
- validateDependency(dependency, ctx);
- collectedDeps.push(dependency);
+ return;
+ }
+
+ /**
+ * For external (npm) packages we need to match them as accurately and granularly as possible:
+ * - If the package.json specifies an exact version, we can match that directly.
+ * - If the package.json specifies something other than an exact version, we need to look up
+ * possible matching nodes from the graph.
+ * - If we have not found any matches yet, we may still have a node containing only the package name
+ * as a fallback.
+ */
+
+ const externalWithExactVersion = `npm:${d}@${version}`;
+
+ if (ctx.externalNodes[externalWithExactVersion]) {
+ applyDependencyTarget(externalWithExactVersion);
+ return;
+ }
+
+ const relevantExternalNodes = depToRelevantExternalNodes.get(d);
+ if (relevantExternalNodes?.length > 0) {
+ // Nodes will already be in the order of highest version first, so the match should be most appropriate
+ for (const node of relevantExternalNodes) {
+ // see if the version is compatible
+ const [_, externalDepVersion] = node.split('@');
+ if (satisfies(externalDepVersion, version)) {
+ applyDependencyTarget(node);
+ return;
+ }
+ }
+ }
+
+ const externalWithNoVersion = `npm:${d}`;
+ if (ctx.externalNodes[externalWithNoVersion]) {
+ applyDependencyTarget(externalWithNoVersion);
+ return;
  }
  });
  } catch (e) {
@@ -99,10 +165,26 @@ function processPackageJson(
 }
 
 function readDeps(packageJson: PackageJson) {
- return [
- ...Object.keys(packageJson?.dependencies ?? {}),
- ...Object.keys(packageJson?.devDependencies ?? {}),
- ...Object.keys(packageJson?.peerDependencies ?? {}),
- ...Object.keys(packageJson?.optionalDependencies ?? {}),
- ];
+ const deps: Record<string, string> = {};
+
+ /**
+ * We process dependencies in a rough order of increasing importance such that if a dependency is listed in multiple
+ * sections, the version listed under the "most important" one wins, with production dependencies being the most important.
+ */
+ const depType = [
+ 'optionalDependencies',
+ 'peerDependencies',
+ 'devDependencies',
+ 'dependencies',
+ ] as const;
+
+ for (const type of depType) {
+ for (const [depName, depVersion] of Object.entries(
+ packageJson[type] || {}
+ )) {
+ deps[depName] = depVersion;
+ }
+ }
+
+ return deps;
 }