From 57770bb12ea6d77373f1e9e8e04f6757b440f277 Mon Sep 17 00:00:00 2001 From: Styerp <35037768+Styerp@users.noreply.github.com> Date: Thu, 9 Feb 2023 09:54:29 -0800 Subject: [PATCH] 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* --- packages/@aws-cdk/core/lib/annotations.ts | 31 +++++-------------- .../@aws-cdk/core/test/annotations.test.ts | 21 ++++++++++++- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/packages/@aws-cdk/core/lib/annotations.ts b/packages/@aws-cdk/core/lib/annotations.ts index 1a6f900f4799a..4907f0b7987b0 100644 --- a/packages/@aws-cdk/core/lib/annotations.ts +++ b/packages/@aws-cdk/core/lib/annotations.ts @@ -1,8 +1,6 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; -import { IConstruct, Node } from 'constructs'; - -const DEPRECATIONS_SYMBOL = Symbol.for('@aws-cdk/core.deprecations'); +import { IConstruct } from 'constructs'; /** * Includes API for attaching annotations such as warning messages to constructs. @@ -77,38 +75,23 @@ export class Annotations { // throw if CDK_BLOCK_DEPRECATIONS is set if (process.env.CDK_BLOCK_DEPRECATIONS) { - throw new Error(`${Node.of(this.scope).path}: ${text}`); - } - - // de-dup based on api key - const set = this.deprecationsReported; - if (set.has(api)) { - return; + throw new Error(`${this.scope.node.path}: ${text}`); } this.addWarning(text); - set.add(api); } /** * Adds a message metadata entry to the construct node, to be displayed by the CDK CLI. + * + * Records the message once per construct. * @param level The message level * @param message The message itself */ private addMessage(level: string, message: string) { - Node.of(this.scope).addMetadata(level, message, { stackTrace: this.stackTraces }); - } - - /** - * Returns the set of deprecations reported on this construct. - */ - private get deprecationsReported() { - let set = (this.scope as any)[DEPRECATIONS_SYMBOL]; - if (!set) { - set = new Set(); - Object.defineProperty(this.scope, DEPRECATIONS_SYMBOL, { value: set }); + const isNew = !this.scope.node.metadata.find((x) => x.data === message); + if (isNew) { + this.scope.node.addMetadata(level, message, { stackTrace: this.stackTraces }); } - - return set; } } diff --git a/packages/@aws-cdk/core/test/annotations.test.ts b/packages/@aws-cdk/core/test/annotations.test.ts index 2f8fd0eef12a1..fc5c7430d22a8 100644 --- a/packages/@aws-cdk/core/test/annotations.test.ts +++ b/packages/@aws-cdk/core/test/annotations.test.ts @@ -1,7 +1,7 @@ import { Construct } from 'constructs'; +import { getWarnings } from './util'; import { App, Stack } from '../lib'; import { Annotations } from '../lib/annotations'; -import { getWarnings } from './util'; const restore = process.env.CDK_BLOCK_DEPRECATIONS; @@ -74,4 +74,23 @@ describe('annotations', () => { process.env.CDK_BLOCK_DEPRECATIONS = '1'; 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/); }); + + test('addMessage deduplicates the message on the node level', () => { + const app = new App(); + const stack = new Stack(app, 'S1'); + const c1 = new Construct(stack, 'C1'); + Annotations.of(c1).addWarning('You should know this!'); + Annotations.of(c1).addWarning('You should know this!'); + Annotations.of(c1).addWarning('You should know this!'); + Annotations.of(c1).addWarning('You should know this, too!'); + expect(getWarnings(app.synth())).toEqual([{ + path: '/S1/C1', + message: 'You should know this!', + }, + { + path: '/S1/C1', + message: 'You should know this, too!', + }], + ); + }); });