Skip to content

Commit

Permalink
fix(core): do not cache external dependency hashes if they are not fu…
Browse files Browse the repository at this point in the history 
…lly resolved (#18020)
  • Loading branch information
@skrtheboss
skrtheboss authored Jul 20, 2023
1 parent b7fb3ea commit c3d4bfe
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 49 deletions.
28 changes: 18 additions & 10 deletions packages/nx/src/hasher/task-hasher.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ describe('TaskHasher', () => {
expect(hash.value).toContain('|5.0.0|');
});

it('should hash entire subtree of dependencies', async () => {
it('should hash entire subtree in a deterministic way', async () => {
const createHasher = () =>
new InProcessTaskHasher(
{},
Expand Down Expand Up @@ -1217,47 +1217,54 @@ describe('TaskHasher', () => {
dependencies: {
appA: [
{
source: 'app',
source: 'appA',
target: 'npm:packageA',
type: DependencyType.static,
},
{
source: 'app',
source: 'appA',
target: 'npm:packageB',
type: DependencyType.static,
},
{
source: 'app',
source: 'appA',
target: 'npm:packageC',
type: DependencyType.static,
},
],
appB: [
{
source: 'app',
source: 'appB',
target: 'npm:packageC',
type: DependencyType.static,
},
],
'npm:packageC': [
{
source: 'app',
source: 'npm:packageC',
target: 'npm:packageA',
type: DependencyType.static,
},
{
source: 'app',
source: 'npm:packageC',
target: 'npm:packageB',
type: DependencyType.static,
},
],
'npm:packageB': [
{
source: 'app',
source: 'npm:packageB',
target: 'npm:packageA',
type: DependencyType.static,
},
],
'npm:packageA': [
{
source: 'npm:packageA',
target: 'npm:packageC',
type: DependencyType.static,
},
],
},
},
{
Expand Down Expand Up @@ -1288,15 +1295,16 @@ describe('TaskHasher', () => {

const hasher1 = createHasher();

await computeTaskHash(hasher1, 'appA');
const hasAppA1 = await computeTaskHash(hasher1, 'appA');
const hashAppB1 = await computeTaskHash(hasher1, 'appB');

const hasher2 = createHasher();

const hashAppB2 = await computeTaskHash(hasher2, 'appB');
await computeTaskHash(hasher2, 'appA');
const hasAppA2 = await computeTaskHash(hasher2, 'appA');

expect(hashAppB1).toEqual(hashAppB2);
expect(hasAppA1).toEqual(hasAppA2);
});

it('should not hash when nx:run-commands executor', async () => {
Expand Down
74 changes: 35 additions & 39 deletions packages/nx/src/hasher/task-hasher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ class TaskHasherImpl {
visited
);
} else {
const hash = this.hashExternalDependency(d.source, d.target);
const { hash } = this.hashExternalDependency(d.target);
return {
value: hash,
details: {
Expand Down Expand Up @@ -408,30 +408,21 @@ class TaskHasherImpl {
return partialHashes;
}

private computeExternalDependencyIdentifier(
sourceProjectName: string,
targetProjectName: string
): `${string}->${string}` {
return `${sourceProjectName}->${targetProjectName}`;
}

private hashExternalDependency(
sourceProjectName: string,
targetProjectName: string,
visited = new Set<string>()
): string {
projectName: string,
parentProjects = new Set<string>()
): { fullyResolved: boolean; hash: string } {
// try to retrieve the hash from cache
if (this.externalDepsHashCache[targetProjectName]) {
return this.externalDepsHashCache[targetProjectName];
if (this.externalDepsHashCache[projectName]) {
return {
fullyResolved: true,
hash: this.externalDepsHashCache[projectName],
};
}
visited.add(
this.computeExternalDependencyIdentifier(
sourceProjectName,
targetProjectName
)
);
const node = this.projectGraph.externalNodes[targetProjectName];
parentProjects.add(projectName);
const node = this.projectGraph.externalNodes[projectName];
let partialHash: string;
let fullyResolved = true;
if (node) {
const partialHashes: string[] = [];
if (node.data.hash) {
Expand All @@ -442,19 +433,20 @@ class TaskHasherImpl {
partialHashes.push(node.data.version);
}
// we want to calculate the hash of the entire dependency tree
if (this.projectGraph.dependencies[targetProjectName]) {
this.projectGraph.dependencies[targetProjectName].forEach((d) => {
if (
!visited.has(
this.computeExternalDependencyIdentifier(
targetProjectName,
d.target
)
)
) {
partialHashes.push(
this.hashExternalDependency(targetProjectName, d.target, visited)
if (this.projectGraph.dependencies[projectName]) {
this.projectGraph.dependencies[projectName].forEach((d) => {
if (!parentProjects.has(d.target)) {
const hashResult = this.hashExternalDependency(
d.target,
new Set(parentProjects)
);
partialHashes.push(hashResult.hash);
if (!hashResult.fullyResolved) {
fullyResolved = false;
}
} else {
// NOTE: do not store hash to cache since it is only a partial hash
fullyResolved = false;
}
});
}
Expand All @@ -465,10 +457,14 @@ class TaskHasherImpl {
// this may occur if dependency is not an npm package
// but rather symlinked in node_modules or it's pointing to a remote git repo
// in this case we have no information about the versioning of the given package
partialHash = `__${targetProjectName}__`;
partialHash = `__${projectName}__`;
}

if (fullyResolved) {
this.externalDepsHashCache[projectName] = partialHash;
}
this.externalDepsHashCache[targetProjectName] = partialHash;
return partialHash;

return { fullyResolved, hash: partialHash };
}

private hashTarget(
Expand All @@ -483,7 +479,7 @@ class TaskHasherImpl {
return;
}

let hash;
let hash: string;
// we can only vouch for @nx packages's executor dependencies
// if it's "run commands" or third-party we skip traversing since we have no info what this command depends on
if (
Expand All @@ -493,7 +489,7 @@ class TaskHasherImpl {
const executorPackage = target.executor.split(':')[0];
const executorNodeName =
this.findExternalDependencyNodeName(executorPackage);
hash = this.hashExternalDependency(projectName, executorNodeName);
hash = this.hashExternalDependency(executorNodeName).hash;
} else {
// use command external dependencies if available to construct the hash
const partialHashes: string[] = [];
Expand All @@ -505,7 +501,7 @@ class TaskHasherImpl {
const externalDependencies = input['externalDependencies'];
for (let dep of externalDependencies) {
dep = this.findExternalDependencyNodeName(dep);
partialHashes.push(this.hashExternalDependency(projectName, dep));
partialHashes.push(this.hashExternalDependency(dep).hash);
}
}
}
Expand Down

1 comment on commit c3d4bfe

@vercel
Copy link

@vercel vercel bot commented on c3d4bfe Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

nx-dev – ./

nx-five.vercel.app
nx-dev-nrwl.vercel.app
nx-dev-git-master-nrwl.vercel.app
nx.dev

Please sign in to comment.