From f138a346754a1a55f41aee54daea90381bda98a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Jona=C5=A1?= Date: Fri, 28 Apr 2023 19:08:05 +0200 Subject: [PATCH] fix(core): ensure stale dependencies are pruned in graph (#16533) --- .../project-graph/build-project-graph.spec.ts | 2 +- .../project-graph-builder.spec.ts | 110 ++++++++++++++++++ .../project-graph/project-graph-builder.ts | 28 +++-- 3 files changed, 127 insertions(+), 13 deletions(-) diff --git a/packages/nx/src/project-graph/build-project-graph.spec.ts b/packages/nx/src/project-graph/build-project-graph.spec.ts index 72985f0dc8b8f..6ec97fdfddbd6 100644 --- a/packages/nx/src/project-graph/build-project-graph.spec.ts +++ b/packages/nx/src/project-graph/build-project-graph.spec.ts @@ -232,6 +232,7 @@ describe('project graph', () => { expect(graph.dependencies).toEqual({ api: [{ source: 'api', target: 'npm:express', type: 'static' }], demo: [ + { source: 'demo', target: 'api', type: 'implicit' }, { source: 'demo', target: 'ui', @@ -243,7 +244,6 @@ describe('project graph', () => { target: 'lazy-lib', type: 'dynamic', }, - { source: 'demo', target: 'api', type: 'implicit' }, ], 'demo-e2e': [], 'lazy-lib': [], diff --git a/packages/nx/src/project-graph/project-graph-builder.spec.ts b/packages/nx/src/project-graph/project-graph-builder.spec.ts index 98ccd3fd81530..1c195d11f139f 100644 --- a/packages/nx/src/project-graph/project-graph-builder.spec.ts +++ b/packages/nx/src/project-graph/project-graph-builder.spec.ts @@ -2,6 +2,7 @@ import { ProjectGraphBuilder } from './project-graph-builder'; describe('ProjectGraphBuilder', () => { let builder: ProjectGraphBuilder; + beforeEach(() => { builder = new ProjectGraphBuilder(); builder.addNode({ @@ -209,4 +210,113 @@ describe('ProjectGraphBuilder', () => { target2: [], }); }); + + it('should prune dependencies when removing explicit dependencies from the files', () => { + builder.addImplicitDependency('source', 'target'); + builder.addExternalNode({ + name: 'npm:external', + type: 'npm', + data: { + version: '1.0.0', + packageName: 'external', + }, + }); + builder.addExternalNode({ + name: 'npm:external2', + type: 'npm', + data: { + version: '1.0.0', + packageName: 'external2', + }, + }); + builder.addStaticDependency('npm:external', 'npm:external2'); + builder.addStaticDependency('source', 'npm:external', 'source/index.ts'); + builder.addDynamicDependency('source', 'npm:external2', 'source/second.ts'); + const graph = builder.getUpdatedProjectGraph(); + + expect(graph.dependencies).toMatchInlineSnapshot(` + { + "npm:external": [ + { + "source": "npm:external", + "target": "npm:external2", + "type": "static", + }, + ], + "source": [ + { + "source": "source", + "target": "target", + "type": "implicit", + }, + { + "source": "source", + "target": "npm:external", + "type": "static", + }, + { + "source": "source", + "target": "npm:external2", + "type": "dynamic", + }, + ], + "target": [], + } + `); + expect(graph.nodes['source'].data.files).toMatchInlineSnapshot(` + [ + { + "dependencies": [ + { + "source": "source", + "target": "npm:external", + "type": "static", + }, + ], + "file": "source/index.ts", + }, + { + "dependencies": [ + { + "source": "source", + "target": "npm:external2", + "type": "dynamic", + }, + ], + "file": "source/second.ts", + }, + ] + `); + + const newBuilder = new ProjectGraphBuilder(graph); + // remove static dependency from the file + delete newBuilder.graph.nodes['source'].data.files[0].dependencies; + + const updatedGraph = newBuilder.getUpdatedProjectGraph(); + + expect(updatedGraph.dependencies).toMatchInlineSnapshot(` + { + "npm:external": [ + { + "source": "npm:external", + "target": "npm:external2", + "type": "static", + }, + ], + "source": [ + { + "source": "source", + "target": "target", + "type": "implicit", + }, + { + "source": "source", + "target": "npm:external2", + "type": "dynamic", + }, + ], + "target": [], + } + `); + }); }); diff --git a/packages/nx/src/project-graph/project-graph-builder.ts b/packages/nx/src/project-graph/project-graph-builder.ts index c7721974cfc3c..36773c22966f3 100644 --- a/packages/nx/src/project-graph/project-graph-builder.ts +++ b/packages/nx/src/project-graph/project-graph-builder.ts @@ -88,6 +88,8 @@ export class ProjectGraphBuilder { targetProjectName: string, sourceProjectFile?: string ): void { + // internal nodes must provide sourceProjectFile when creating static dependency + // externalNodes do not have sourceProjectFile if (this.graph.nodes[sourceProjectName] && !sourceProjectFile) { throw new Error(`Source project file is required`); } @@ -110,6 +112,7 @@ export class ProjectGraphBuilder { if (this.graph.externalNodes[sourceProjectName]) { throw new Error(`External projects can't have "dynamic" dependencies`); } + // dynamic dependency is always bound to a file if (!sourceProjectFile) { throw new Error(`Source project file is required`); } @@ -281,10 +284,6 @@ export class ProjectGraphBuilder { const isDuplicate = !!this.graph.dependencies[sourceProjectName].find( (d) => d.target === targetProjectName && d.type === type ); - // do not add duplicate to project - if (isDuplicate && !sourceProjectFile) { - return; - } const dependency = { source: sourceProjectName, @@ -311,16 +310,18 @@ export class ProjectGraphBuilder { if (!fileData.dependencies) { fileData.dependencies = []; } - if (!fileData.dependencies.find((t) => t.target === targetProjectName)) { + if ( + !fileData.dependencies.find( + (t) => t.target === targetProjectName && t.type === type + ) + ) { fileData.dependencies.push(dependency); } + } else if (!isDuplicate) { + // only add to dependencies section if the source file is not specified + // and not already added + this.graph.dependencies[sourceProjectName].push(dependency); } - - if (isDuplicate) { - return; - } - - this.graph.dependencies[sourceProjectName].push(dependency); } private removeDependenciesWithNode(name: string) { @@ -376,7 +377,10 @@ export class ProjectGraphBuilder { if (this.graph.dependencies[sourceProject]) { const removed = this.removedEdges[sourceProject]; for (const d of this.graph.dependencies[sourceProject]) { - if (!removed || !removed.has(d.target)) { + // static and dynamic dependencies of internal projects + // will be rebuilt based on the file dependencies + // we only need to keep the implicit dependencies + if (d.type === DependencyType.implicit && !removed?.has(d.target)) { if (!alreadySetTargetProjects.has(d.target)) { alreadySetTargetProjects.set(d.target, new Map([[d.type, d]])); } else {