fix(core): ensure external dependency hashes are resolved in a determ…

…inistic way External dependency hashes were not deterministic and could cause cache misses, since the hash was dependent on which task was hashed when. This was caused by the externalDepsHashCache which was filled with values which are dependent on the visited set. Fixes #17917
nrwl · Jul 3, 2023 · 879e3ed · 879e3ed
1 parent a70e1a8
commit 879e3ed
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 7 deletions.
diff --git a/packages/nx/src/hasher/task-hasher.spec.ts b/packages/nx/src/hasher/task-hasher.spec.ts
@@ -1161,6 +1161,144 @@ describe('TaskHasher', () => {
       expect(hash.value).toContain('|5.0.0|');
     });
 
+    it('should hash entire subtree of dependencies', async () => {
+      const createHasher = () =>
+        new InProcessTaskHasher(
+          {},
+          allWorkspaceFiles,
+          {
+            nodes: {
+              appA: {
+                name: 'appA',
+                type: 'app',
+                data: {
+                  root: 'apps/appA',
+                  targets: { build: { executor: '@nx/webpack:webpack' } },
+                },
+              },
+              appB: {
+                name: 'appB',
+                type: 'app',
+                data: {
+                  root: 'apps/appB',
+                  targets: { build: { executor: '@nx/webpack:webpack' } },
+                },
+              },
+            },
+            externalNodes: {
+              'npm:packageA': {
+                name: 'npm:packageA',
+                type: 'npm',
+                data: {
+                  packageName: 'packageA',
+                  version: '0.0.0',
+                  hash: '$packageA0.0.0$',
+                },
+              },
+              'npm:packageB': {
+                name: 'npm:packageB',
+                type: 'npm',
+                data: {
+                  packageName: 'packageB',
+                  version: '0.0.0',
+                  hash: '$packageB0.0.0$',
+                },
+              },
+              'npm:packageC': {
+                name: 'npm:packageC',
+                type: 'npm',
+                data: {
+                  packageName: 'packageC',
+                  version: '0.0.0',
+                  hash: '$packageC0.0.0$',
+                },
+              },
+            },
+            dependencies: {
+              appA: [
+                {
+                  source: 'app',
+                  target: 'npm:packageA',
+                  type: DependencyType.static,
+                },
+                {
+                  source: 'app',
+                  target: 'npm:packageB',
+                  type: DependencyType.static,
+                },
+                {
+                  source: 'app',
+                  target: 'npm:packageC',
+                  type: DependencyType.static,
+                },
+              ],
+              appB: [
+                {
+                  source: 'app',
+                  target: 'npm:packageC',
+                  type: DependencyType.static,
+                },
+              ],
+              'npm:packageC': [
+                {
+                  source: 'app',
+                  target: 'npm:packageA',
+                  type: DependencyType.static,
+                },
+                {
+                  source: 'app',
+                  target: 'npm:packageB',
+                  type: DependencyType.static,
+                },
+              ],
+              'npm:packageB': [
+                {
+                  source: 'app',
+                  target: 'npm:packageA',
+                  type: DependencyType.static,
+                },
+              ],
+            },
+          },
+          {
+            roots: ['app-build'],
+            tasks: {
+              'app-build': {
+                id: 'app-build',
+                target: { project: 'app', target: 'build' },
+                overrides: {},
+              },
+            },
+            dependencies: {},
+          },
+          {} as any,
+          {},
+          fileHasher
+        );
+
+      const computeTaskHash = async (hasher, appName) => {
+        const hashAppA = await hasher.hashTask({
+          target: { project: appName, target: 'build' },
+          id: `${appName}-build`,
+          overrides: { prop: 'prop-value' },
+        });
+
+        return hashAppA.value;
+      };
+
+      const hasher1 = createHasher();
+
+      await computeTaskHash(hasher1, 'appA');
+      const hashAppB1 = await computeTaskHash(hasher1, 'appB');
+
+      const hasher2 = createHasher();
+
+      const hashAppB2 = await computeTaskHash(hasher2, 'appB');
+      await computeTaskHash(hasher2, 'appA');
+
+      expect(hashAppB1).toEqual(hashAppB2);
+    });
+
     it('should not hash when nx:run-commands executor', async () => {
       const hasher = new InProcessTaskHasher(
         {},

diff --git a/packages/nx/src/hasher/task-hasher.ts b/packages/nx/src/hasher/task-hasher.ts
@@ -429,13 +429,14 @@ class TaskHasherImpl {
         partialHashes.push(node.data.version);
       }
       // we want to calculate the hash of the entire dependency tree
-      if (this.projectGraph.dependencies[projectName]) {
-        this.projectGraph.dependencies[projectName].forEach((d) => {
-          if (!visited.has(d.target)) {
-            partialHashes.push(this.hashExternalDependency(d.target, visited));
-          }
-        });
-      }
+      this.projectGraph.dependencies[projectName]?.forEach((d) => {
+        if (!visited.has(d.target)) {
+          partialHashes.push(
+            this.hashExternalDependency(d.target, new Set(visited))
+          );
+        }
+      });
+
       partialHash = hashArray(partialHashes);
     } else {
       // unknown dependency