Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): messages are displayed multiple times per construct #24019

Merged
merged 2 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 7 additions & 24 deletions packages/@aws-cdk/core/lib/annotations.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
}
}
21 changes: 20 additions & 1 deletion packages/@aws-cdk/core/test/annotations.test.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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!',
}],
);
});
});