fix(core): rework how dev dependencies are ignored from target packag…

…e.json
nrwl · Jul 4, 2023 · 64618b0 · 64618b0
1 parent 87866f5
commit 64618b0
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 38 deletions.
diff --git a/docs/generated/devkit/nx_devkit.md b/docs/generated/devkit/nx_devkit.md
@@ -39,6 +39,7 @@ It only uses language primitives and immutable objects
 - [NxAffectedConfig](../../devkit/documents/nx_devkit#nxaffectedconfig)
 - [NxJsonConfiguration](../../devkit/documents/nx_devkit#nxjsonconfiguration)
 - [NxPlugin](../../devkit/documents/nx_devkit#nxplugin)
+- [PackageJson](../../devkit/documents/nx_devkit#packagejson)
 - [ProjectConfiguration](../../devkit/documents/nx_devkit#projectconfiguration)
 - [ProjectFileMap](../../devkit/documents/nx_devkit#projectfilemap)
 - [ProjectGraph](../../devkit/documents/nx_devkit#projectgraph)
@@ -311,6 +312,14 @@ A plugin for Nx
 
 ---
 
+### PackageJson
+
+• **PackageJson**: `Object`
+
+Package Json configuration
+
+---
+
 ### ProjectConfiguration
 
 • **ProjectConfiguration**: `Object`

diff --git a/docs/generated/packages/devkit/documents/nx_devkit.md b/docs/generated/packages/devkit/documents/nx_devkit.md
@@ -39,6 +39,7 @@ It only uses language primitives and immutable objects
 - [NxAffectedConfig](../../devkit/documents/nx_devkit#nxaffectedconfig)
 - [NxJsonConfiguration](../../devkit/documents/nx_devkit#nxjsonconfiguration)
 - [NxPlugin](../../devkit/documents/nx_devkit#nxplugin)
+- [PackageJson](../../devkit/documents/nx_devkit#packagejson)
 - [ProjectConfiguration](../../devkit/documents/nx_devkit#projectconfiguration)
 - [ProjectFileMap](../../devkit/documents/nx_devkit#projectfilemap)
 - [ProjectGraph](../../devkit/documents/nx_devkit#projectgraph)
@@ -311,6 +312,14 @@ A plugin for Nx
 
 ---
 
+### PackageJson
+
+• **PackageJson**: `Object`
+
+Package Json configuration
+
+---
+
 ### ProjectConfiguration
 
 • **ProjectConfiguration**: `Object`

diff --git a/packages/eslint-plugin/src/rules/dependency-checks.ts b/packages/eslint-plugin/src/rules/dependency-checks.ts
@@ -9,6 +9,7 @@ import { satisfies } from 'semver';
 import { getHelperDependenciesFromProjectGraph } from '@nx/js';
 import {
   getAllDependencies,
+  getPackageJson,
   removePackageJsonFromFileMap,
 } from '../utils/package-json-utils';
 
@@ -125,13 +126,17 @@ export default createESLintRule<Options, MessageIds>({
       projectGraph
     );
 
+    const rootPackageJson = getPackageJson(join(workspaceRoot, 'package.json'));
+
     // find all dependencies for the project
     const npmDeps = findProjectsNpmDependencies(
       sourceProject,
       projectGraph,
       buildTarget,
+      rootPackageJson,
       {
         helperDependencies: helperDependencies.map((dep) => dep.target),
+        isProduction: true,
       },
       removePackageJsonFromFileMap(projectFileMap)
     );
@@ -147,12 +152,12 @@ export default createESLintRule<Options, MessageIds>({
       'package.json'
     );
 
-    globalThis.projPackageJsonDeps ??= getAllDependencies(projPackageJsonPath);
+    globalThis.projPackageJsonDeps ??= getAllDependencies(
+      getPackageJson(projPackageJsonPath)
+    );
     const projPackageJsonDeps: Record<string, string> =
       globalThis.projPackageJsonDeps;
-    const rootPackageJsonDeps = getAllDependencies(
-      join(workspaceRoot, 'package.json')
-    );
+    const rootPackageJsonDeps = getAllDependencies(rootPackageJson);
 
     function validateMissingDependencies(node: AST.JSONProperty) {
       if (!checkMissingDependencies) {
@@ -261,15 +266,6 @@ export default createESLintRule<Options, MessageIds>({
               ]);
             }
           }
-
-          // remove 4 spaces, new line and potential comma from previous line
-          const shouldRemoveSiblingComma =
-            isLastProperty && node.parent.properties.length > 1;
-          const leadingChars = 5 + (shouldRemoveSiblingComma ? 1 : 0);
-          return fixer.removeRange([
-            node.range[0] - leadingChars,
-            node.range[1] + (isLastProperty ? 0 : 1),
-          ]);
         },
       });
     }

diff --git a/packages/eslint-plugin/src/utils/package-json-utils.ts b/packages/eslint-plugin/src/utils/package-json-utils.ts
@@ -1,16 +1,21 @@
-import { ProjectFileMap, readJsonFile } from '@nx/devkit';
+import { PackageJson, ProjectFileMap, readJsonFile } from '@nx/devkit';
 import { existsSync } from 'fs';
 
-export function getAllDependencies(path: string): Record<string, string> {
+export function getAllDependencies(
+  packageJson: PackageJson
+): Record<string, string> {
+  return {
+    ...packageJson.dependencies,
+    ...packageJson.devDependencies,
+    ...packageJson.peerDependencies,
+  };
+}
+
+export function getPackageJson(path: string): PackageJson {
   if (existsSync(path)) {
-    const packageJson = readJsonFile(path);
-    return {
-      ...packageJson.dependencies,
-      ...packageJson.devDependencies,
-      ...packageJson.peerDependencies,
-    };
+    return readJsonFile(path);
   }
-  return {};
+  return {} as PackageJson;
 }
 
 export function removePackageJsonFromFileMap(

diff --git a/packages/nx/src/devkit-exports.ts b/packages/nx/src/devkit-exports.ts
@@ -174,6 +174,11 @@ export { readJsonFile, writeJsonFile } from './utils/fileutils';
  */
 export { stripIndents } from './utils/strip-indents';
 
+/**
+ * @category Utils
+ */
+export type { PackageJson } from './utils/package-json';
+
 /**
  * @category Utils
  */

diff --git a/packages/nx/src/plugins/js/package-json/create-package-json.ts b/packages/nx/src/plugins/js/package-json/create-package-json.ts
@@ -16,7 +16,6 @@ import {
 import { readNxJson } from '../../../config/configuration';
 import { readProjectFileMapCache } from '../../../project-graph/nx-deps-cache';
 import { join } from 'path';
-import { file } from 'tmp';
 
 interface NpmDeps {
   readonly dependencies: Record<string, string>;
@@ -44,11 +43,19 @@ export function createPackageJson(
   const projectNode = graph.nodes[projectName];
   const isLibrary = projectNode.type === 'lib';
 
+  const rootPackageJson = readJsonFile(
+    `${options.root || workspaceRoot}/package.json`
+  );
+
   const npmDeps = findProjectsNpmDependencies(
     projectNode,
     graph,
     options.target,
-    { helperDependencies: options.helperDependencies },
+    rootPackageJson,
+    {
+      helperDependencies: options.helperDependencies,
+      isProduction: options.isProduction,
+    },
     fileMap
   );
 
@@ -93,9 +100,6 @@ export function createPackageJson(
     );
   };
 
-  const rootPackageJson = readJsonFile(
-    `${options.root || workspaceRoot}/package.json`
-  );
   Object.entries(npmDeps.dependencies).forEach(([packageName, version]) => {
     if (
       rootPackageJson.devDependencies?.[packageName] &&
@@ -180,9 +184,11 @@ export function findProjectsNpmDependencies(
   projectNode: ProjectGraphProjectNode,
   graph: ProjectGraph,
   target: string,
+  rootPackageJson: PackageJson,
   options: {
     helperDependencies?: string[];
     ignoredDependencies?: string[];
+    isProduction?: boolean;
   },
   fileMap?: ProjectFileMap
 ): NpmDeps {
@@ -209,29 +215,29 @@ export function findProjectsNpmDependencies(
     recursivelyCollectPeerDependencies(dep, graph, npmDeps, seen);
   });
 
+  // if it's production, we want to ignore all found devDependencies
+  const ignoredDependencies =
+    options.isProduction && rootPackageJson.devDependencies
+      ? [
+          ...(options.ignoredDependencies || []),
+          ...Object.keys(rootPackageJson.devDependencies),
+        ]
+      : options.ignoredDependencies || [];
+
   findAllNpmDeps(
     fileMap,
     projectNode,
     graph,
     npmDeps,
     seen,
-    options.ignoredDependencies || [],
+    ignoredDependencies,
     dependencyInputs,
     selfInputs
   );
 
   return npmDeps;
 }
 
-// this function checks if the file path in in the root and not index.ts file
-// these files are likely configuration files
-function isMainOrNonRoot(filePath: string, root: string): boolean {
-  if (filePath.slice(root.length + 1).includes('/')) {
-    return true;
-  }
-  return !!/(index|main|public(-|_)api)\.(t|j)sx?$/.exec(filePath);
-}
-
 function findAllNpmDeps(
   projectFileMap: ProjectFileMap,
   projectNode: ProjectGraphProjectNode,
@@ -250,7 +256,7 @@ function findAllNpmDeps(
     projectNode.data.root,
     projectFileMap[projectNode.name] || [],
     rootPatterns ?? dependencyPatterns
-  ).filter((f) => isMainOrNonRoot(f.file, projectNode.data.root));
+  );
 
   const projectDependencies = new Set<string>();
 

diff --git a/packages/nx/src/utils/package-json.ts b/packages/nx/src/utils/package-json.ts
@@ -33,6 +33,9 @@ export interface NxMigrationsConfiguration {
 
 type PackageOverride = { [key: string]: string | PackageOverride };
 
+/**
+ * Package Json configuration
+ */
 export interface PackageJson {
   // Generic Package.Json Configuration
   name: string;