Merge pull request #311 from snyk/fix/osm-3760-yarn-workspace-dev-deps

fix(yarn-berry): prune workspace devDependencies from prod graph

Author: JamesPatrickGill

lib/dep-graph-builders/types.ts

@@ -62,6 +62,19 @@ export type YarnLockV2WorkspaceArgs = {
   isWorkspacePkg: boolean;
   isRoot: boolean;
   rootResolutions: Record<string, string>;
+  // Map of workspace package name -> that member's package.json dependency groups.
+  // Yarn Berry merges a workspace package's dependencies + devDependencies into a single
+  // `dependencies` block in yarn.lock, dropping the dev marker. When a workspace package is
+  // consumed as a production dependency we use this map to prune its dev-only dependencies
+  // so they are not promoted into the production graph.
+  workspacePackages?: Record<string, WorkspacePackageManifest>;
+};
+
+export type WorkspacePackageManifest = {
+  dependencies?: Record<string, string>;
+  devDependencies?: Record<string, string>;
+  optionalDependencies?: Record<string, string>;
+  peerDependencies?: Record<string, string>;
 };
 
 export type YarnLockV2ProjectParseOptions = {

lib/dep-graph-builders/yarn-lock-v2/build-depgraph-simple.ts

@@ -53,6 +53,8 @@ export const buildDepGraphYarnLockV2Simple = async (
     pruneWithinTopLevelDeps,
     undefined,
     showNpmScope,
+    includeDevDeps,
+    workspaceArgs?.workspacePackages,
   );
 
   return depGraphBuilder.build();
@@ -74,6 +76,8 @@ const dfsVisit = async (
   pruneWithinTopLevel: boolean,
   visited?: Set<string>,
   showNpmScope?: boolean,
+  includeDevDeps = false,
+  workspacePackages?: YarnLockV2WorkspaceArgs['workspacePackages'],
 ): Promise<void> => {
   for (const [name, depInfo] of Object.entries(node.dependencies || {})) {
     let scopeDepInfo = depInfo;
@@ -111,6 +115,8 @@ const dfsVisit = async (
       includeOptionalDeps,
       resolutions,
       node,
+      includeDevDeps,
+      workspacePackages,
     );
 
     if (localVisited.has(childNode.id)) {
@@ -174,6 +180,8 @@ const dfsVisit = async (
       pruneWithinTopLevel,
       localVisited,
       showNpmScope,
+      includeDevDeps,
+      workspacePackages,
     );
   }
 };

lib/dep-graph-builders/yarn-lock-v2/utils.ts

@@ -2,8 +2,8 @@ import { structUtils } from '@yarnpkg/core';
 import * as _flatMap from 'lodash.flatmap';
 import { OutOfSyncError } from '../../errors';
 import { LockfileType } from '../../parsers';
-import { NormalisedPkgs } from '../types';
-import { getGraphDependencies, PkgNode } from '../util';
+import { NormalisedPkgs, WorkspacePackageManifest } from '../types';
+import { Dependencies, getGraphDependencies, PkgNode } from '../util';
 import * as semver from 'semver';
 import * as debugModule from 'debug';
 
@@ -99,6 +99,39 @@ export const yarnLockFileKeyNormalizer =
     return new Set<string>(_flatMap(allKeys));
   };
 
+/**
+ * Yarn Berry merges a workspace package's dependencies, devDependencies and peerDependencies
+ * into a single `dependencies` block in yarn.lock, so the dev marker is lost. When such a
+ * workspace package is consumed as a production dependency, walking that merged block would
+ * promote its dev-only tooling (e.g. webpack, babel) into the production graph.
+ *
+ * Given the consumed workspace member's own package.json (via `workspaceManifest`), drop the
+ * dev-only entries: names that appear in devDependencies but not in dependencies /
+ * optionalDependencies / peerDependencies (prod wins on overlap).
+ */
+const pruneWorkspaceDevDependencies = (
+  deps: Dependencies,
+  workspaceManifest: WorkspacePackageManifest,
+): Dependencies => {
+  const nonDev = new Set<string>([
+    ...Object.keys(workspaceManifest.dependencies || {}),
+    ...Object.keys(workspaceManifest.optionalDependencies || {}),
+    ...Object.keys(workspaceManifest.peerDependencies || {}),
+  ]);
+  const dev = new Set<string>(
+    Object.keys(workspaceManifest.devDependencies || {}),
+  );
+
+  return Object.entries(deps).reduce((acc: Dependencies, [name, depInfo]) => {
+    // Drop only names that are exclusively devDependencies of the workspace member.
+    if (dev.has(name) && !nonDev.has(name)) {
+      return acc;
+    }
+    acc[name] = depInfo;
+    return acc;
+  }, {});
+};
+
 export const getYarnLockV2ChildNode = (
   name: string,
   depInfo: {
@@ -116,6 +149,8 @@ export const getYarnLockV2ChildNode = (
   includeOptionalDeps: boolean,
   resolutions: Record<string, string>,
   parentNode: PkgNode,
+  includeDevDeps = false,
+  workspacePackages?: Record<string, WorkspacePackageManifest>,
 ) => {
   // First, check if a resolution would be used
   const resolvedVersionFromResolution = (() => {
@@ -256,7 +291,7 @@ export const getYarnLockV2ChildNode = (
       optionalDependencies,
     } = pkgData;
 
-    const formattedDependencies = getGraphDependencies(dependencies || {}, {
+    let formattedDependencies = getGraphDependencies(dependencies || {}, {
       isDev: depInfo.isDev,
     });
     const formattedOptionalDependencies = includeOptionalDeps
@@ -266,6 +301,23 @@ export const getYarnLockV2ChildNode = (
         })
       : {};
 
+    // Key by the resolved package name, not the parent's name, so npm aliases
+    // (e.g. "alias": "npm:@scope/real-pkg@1") still match the workspace manifest.
+    const workspaceManifestFromResolution =
+      workspacePackages?.[
+        depInfo.alias ? depInfo.alias.aliasTargetDepName : name
+      ];
+    if (
+      !includeDevDeps &&
+      workspaceManifestFromResolution &&
+      pkgData.resolution?.includes('@workspace:')
+    ) {
+      formattedDependencies = pruneWorkspaceDevDependencies(
+        formattedDependencies,
+        workspaceManifestFromResolution,
+      );
+    }
+
     return {
       id: `${name}@${versionFromResolution}`,
       name: depInfo.alias ? depInfo.alias.aliasTargetDepName : name,
@@ -301,7 +353,7 @@ export const getYarnLockV2ChildNode = (
     }
   } else {
     const depData = pkgs[childNodeKey];
-    const dependencies = getGraphDependencies(depData.dependencies || {}, {
+    let dependencies = getGraphDependencies(depData.dependencies || {}, {
       isDev: depInfo.isDev,
     });
     const optionalDependencies = includeOptionalDeps
@@ -310,6 +362,24 @@ export const getYarnLockV2ChildNode = (
           isOptional: true,
         })
       : {};
+
+    // Key by the resolved package name, not the parent's name, so npm aliases
+    // (e.g. "alias": "npm:@scope/real-pkg@1") still match the workspace manifest.
+    const workspaceManifest =
+      workspacePackages?.[
+        depInfo.alias ? depInfo.alias.aliasTargetDepName : name
+      ];
+    if (
+      !includeDevDeps &&
+      workspaceManifest &&
+      depData.resolution?.includes('@workspace:')
+    ) {
+      dependencies = pruneWorkspaceDevDependencies(
+        dependencies,
+        workspaceManifest,
+      );
+    }
+
     return {
       id: `${name}@${depData.version}`,
       name: depInfo.alias ? depInfo.alias.aliasTargetDepName : name,

lib/index.ts

@@ -62,6 +62,7 @@ import type {
   NormalisedPkgs,
   ProjectParseOptions,
   YarnLockV2ProjectParseOptions,
+  WorkspacePackageManifest,
 } from './dep-graph-builders/types';
 import {
   getLockfileVersionFromFile,
@@ -92,6 +93,7 @@ export {
   PackageJsonBase,
   ProjectParseOptions,
   YarnLockV2ProjectParseOptions,
+  WorkspacePackageManifest,
   NormalisedPkgs,
   NormalisedPkgs as YarnLockPackages,
   getLockfileVersionFromFile,

test/jest/dep-graph-builders/fixtures/yarn-lock-v2/real/workspace-dev-deps/apps/my-app/package.json

@@ -0,0 +1,24 @@
+{
+  "name": "@demo/my-app",
+  "version": "0.0.0-development",
+  "author": "Demo Team",
+  "files": [
+    "dist",
+    "src"
+  ],
+  "main": "./src/index.js",
+  "scripts": {
+    "build": "webpack --config webpack.config.js",
+    "test": "jest",
+    "package:clean": "CLEAN_PACKAGE=$(jq --unbuffered 'setpath([\"devDependencies\"]; .devDependencies * (.dependencies // {})) | del(.dependencies)' < ./package.json); printf '%s' \"$CLEAN_PACKAGE\" > ./package.json"
+  },
+  "devDependencies": {
+    "@demo/private-lib": "workspace:^",
+    "eslint": "^8",
+    "webpack": "^5.90.0",
+    "webpack-cli": "^5.1.4"
+  },
+  "dependencies": {
+    "@demo/shared-lib": "*"
+  }
+}

test/jest/dep-graph-builders/fixtures/yarn-lock-v2/real/workspace-dev-deps/libraries/private-lib/package.json

@@ -0,0 +1,25 @@
+{
+  "name": "@demo/private-lib",
+  "version": "0.0.1",
+  "private": true,
+  "author": "Demo Team",
+  "files": [
+    "src",
+    "dist"
+  ],
+  "main": "./dist/index.js",
+  "scripts": {
+    "build": "webpack --config webpack.config.js",
+    "test": "jest"
+  },
+  "dependencies": {
+    "semver": "^7.6.0"
+  },
+  "devDependencies": {
+    "@babel/core": "^7.24.0",
+    "@babel/preset-env": "^7.24.0",
+    "babel-loader": "^9.1.3",
+    "webpack": "^5.90.0",
+    "webpack-cli": "^5.1.4"
+  }
+}

test/jest/dep-graph-builders/fixtures/yarn-lock-v2/real/workspace-dev-deps/libraries/shared-lib/package.json

@@ -0,0 +1,21 @@
+{
+  "name": "@demo/shared-lib",
+  "version": "0.0.1",
+  "author": "Demo Team",
+  "files": [
+    "src",
+    "dist"
+  ],
+  "main": "./dist/index.js",
+  "scripts": {
+    "build": "webpack --config webpack.config.js",
+    "test": "jest"
+  },
+  "devDependencies": {
+    "@babel/core": "^7.24.0",
+    "@babel/preset-env": "^7.24.0",
+    "babel-loader": "^9.1.3",
+    "webpack": "^5.90.0",
+    "webpack-cli": "^5.1.4"
+  }
+}

test/jest/dep-graph-builders/fixtures/yarn-lock-v2/real/workspace-dev-deps/package.json

@@ -0,0 +1,21 @@
+{
+  "name": "@demo/workspace-root",
+  "packageManager": "yarn@4.3.0",
+  "version": "0.0.0-development",
+  "private": true,
+  "workspaces": [
+    "apps/*",
+    "libraries/*"
+  ],
+  "scripts": {
+    "build:all": "yarn workspaces foreach -Avp --topological-dev run build",
+    "test:all": "yarn workspaces foreach -Avp --topological-dev run test"
+  },
+  "devDependencies": {
+    "eslint": "^8",
+    "typescript": "^5"
+  },
+  "resolutions": {
+    "webpack": "^5.90.0"
+  }
+}