Skip to content

Commit 57770bb

Browse files
authored
fix(core): messages are displayed multiple times per construct (#24019)
Annotations added many times clog terminal space and make debugging difficult. Deduplicate annotations based on the message. Closes #9565. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 5942978 commit 57770bb

File tree

2 files changed

+27
-25
lines changed

2 files changed

+27
-25
lines changed

packages/@aws-cdk/core/lib/annotations.ts

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
22
import * as cxapi from '@aws-cdk/cx-api';
3-
import { IConstruct, Node } from 'constructs';
4-
5-
const DEPRECATIONS_SYMBOL = Symbol.for('@aws-cdk/core.deprecations');
3+
import { IConstruct } from 'constructs';
64

75
/**
86
* Includes API for attaching annotations such as warning messages to constructs.
@@ -77,38 +75,23 @@ export class Annotations {
7775

7876
// throw if CDK_BLOCK_DEPRECATIONS is set
7977
if (process.env.CDK_BLOCK_DEPRECATIONS) {
80-
throw new Error(`${Node.of(this.scope).path}: ${text}`);
81-
}
82-
83-
// de-dup based on api key
84-
const set = this.deprecationsReported;
85-
if (set.has(api)) {
86-
return;
78+
throw new Error(`${this.scope.node.path}: ${text}`);
8779
}
8880

8981
this.addWarning(text);
90-
set.add(api);
9182
}
9283

9384
/**
9485
* Adds a message metadata entry to the construct node, to be displayed by the CDK CLI.
86+
*
87+
* Records the message once per construct.
9588
* @param level The message level
9689
* @param message The message itself
9790
*/
9891
private addMessage(level: string, message: string) {
99-
Node.of(this.scope).addMetadata(level, message, { stackTrace: this.stackTraces });
100-
}
101-
102-
/**
103-
* Returns the set of deprecations reported on this construct.
104-
*/
105-
private get deprecationsReported() {
106-
let set = (this.scope as any)[DEPRECATIONS_SYMBOL];
107-
if (!set) {
108-
set = new Set();
109-
Object.defineProperty(this.scope, DEPRECATIONS_SYMBOL, { value: set });
92+
const isNew = !this.scope.node.metadata.find((x) => x.data === message);
93+
if (isNew) {
94+
this.scope.node.addMetadata(level, message, { stackTrace: this.stackTraces });
11095
}
111-
112-
return set;
11396
}
11497
}

packages/@aws-cdk/core/test/annotations.test.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Construct } from 'constructs';
2+
import { getWarnings } from './util';
23
import { App, Stack } from '../lib';
34
import { Annotations } from '../lib/annotations';
4-
import { getWarnings } from './util';
55

66
const restore = process.env.CDK_BLOCK_DEPRECATIONS;
77

@@ -74,4 +74,23 @@ describe('annotations', () => {
7474
process.env.CDK_BLOCK_DEPRECATIONS = '1';
7575
expect(() => Annotations.of(c1).addDeprecation('foo', 'bar')).toThrow(/MyStack\/Hello: The API foo is deprecated: bar\. This API will be removed in the next major release/);
7676
});
77+
78+
test('addMessage deduplicates the message on the node level', () => {
79+
const app = new App();
80+
const stack = new Stack(app, 'S1');
81+
const c1 = new Construct(stack, 'C1');
82+
Annotations.of(c1).addWarning('You should know this!');
83+
Annotations.of(c1).addWarning('You should know this!');
84+
Annotations.of(c1).addWarning('You should know this!');
85+
Annotations.of(c1).addWarning('You should know this, too!');
86+
expect(getWarnings(app.synth())).toEqual([{
87+
path: '/S1/C1',
88+
message: 'You should know this!',
89+
},
90+
{
91+
path: '/S1/C1',
92+
message: 'You should know this, too!',
93+
}],
94+
);
95+
});
7796
});

0 commit comments

Comments
 (0)