Skip to content

Commit 2b0b5ea

Browse files
authored
fix(pipelines): graphnode dependencies can have duplicates (#18450)
`GraphNode.allDeps` allows duplicate dependencies to be returned. This does not have any affect on the performance of the pipelines module, but looks ugly. This was noticed in cdklabs/cdk-pipelines-github#67, where the dependencies are written out to the `deploy.yaml` file. I did not change the underlying `GraphNode.dependencies` structure to be a set (although I think it should) because I feel like that is a breaking change. So instead I've preserved the structure of the API and deduplicated the list of GraphNodes. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent ab4a7ad commit 2b0b5ea

File tree

2 files changed

+21
-2
lines changed

2 files changed

+21
-2
lines changed

packages/@aws-cdk/pipelines/lib/helpers-internal/graph.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export class GraphNode<A> {
3535
*/
3636
public get allDeps(): GraphNode<A>[] {
3737
const fromParent = this.parentGraph?.allDeps ?? [];
38-
return [...this.dependencies, ...fromParent];
38+
return Array.from(new Set([...this.dependencies, ...fromParent]));
3939
}
4040

4141
public dependOn(...dependencies: Array<GraphNode<A> | undefined>) {
@@ -382,4 +382,4 @@ function projectDependencies<A>(dependencies: Map<GraphNode<A>, Set<GraphNode<A>
382382

383383
export function isGraph<A>(x: GraphNode<A>): x is Graph<A> {
384384
return x instanceof Graph;
385-
}
385+
}

packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/dependencies.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,22 @@ describe('with nested graphs', () => {
5050
]);
5151
});
5252
});
53+
54+
test('duplicate dependencies are ignored', () => {
55+
mkGraph('G', G => {
56+
const A = G.graph('A', [], GA => {
57+
GA.node('aa');
58+
});
59+
60+
// parent graph depnds on A also
61+
const B = G.graph('B', [A], GB => {
62+
// duplicate dependency on A
63+
GB.graph('BB', [A], GBB => {
64+
GBB.node('bbb');
65+
});
66+
GB.node('bb');
67+
});
68+
69+
expect(nodeNames(B.tryGetChild('BB')!.allDeps)).toStrictEqual(['A']);
70+
});
71+
});

0 commit comments

Comments
 (0)