Skip to content

Commit

Permalink
feat(core): facility to warn when using deprecated apis
Browse files Browse the repository at this point in the history
Introduce `Annotations.addDeprecation()` which will attach a warning to the construct indicating that a deprecated API is used.

At the moment, we only use this to warn when `.node` is used instead of `.construct`, but we will gradually use this to report the usage of all deprecated APIs as a preparation for v2.0.

If the environment variable `CDK_BLOCK_DEPRECATIONS` is set (and it is set in `cdk-test`), it will cause usage of deprecated APIs to throw an error instead.

Related: aws/aws-cdk-rfcs#192
  • Loading branch information
Elad Ben-Israel committed Aug 11, 2020
1 parent 1fe9684 commit 006954d
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 13 deletions.
49 changes: 48 additions & 1 deletion packages/@aws-cdk/core/lib/annotations.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import { IConstruct } from './construct-compat';

const DEPRECATIONS_SYMBOL = Symbol.for('@aws-cdk/core.deprecations');

/**
* Includes API for attaching annotations such as warning messages to constructs.
*/
Expand Down Expand Up @@ -49,6 +51,38 @@ export class Annotations {
this.addMessage(cxschema.ArtifactMetadataEntryType.ERROR, message);
}

/**
* Adds a deprecation warning for a specific API.
*
* Deprecations will be added only once per construct as a warning and will be
* deduplicated based on the `api`.
*
* If the environment variable `CDK_BLOCK_DEPRECATIONS` is set, this method
* will throw an error instead with the deprecation message.
*
* @param api The API being deprecated in the format `module.Class.property`
* (e.g. `@aws-cdk/core.Construct.node`).
* @param message The deprecation message to display, with information about
* alternatives.
*/
public addDeprecation(api: string, message: string) {
const text = `The API ${api} is deprecated: ${message}. This API should will be removed in the next major version of the AWS CDK`;

// throw if CDK_BLOCK_DEPRECATIONS is set
if (process.env.CDK_BLOCK_DEPRECATIONS) {
throw new Error(`${this.scope.construct.path}: ${text}`);
}

// de-dup based on api key
const set = this.deprecationsReported;
if (set.has(api)) {
return;
}

this.addWarning(text);
set.add(api);
}

/**
* Adds a message metadata entry to the construct node, to be displayed by the CDK CLI.
* @param level The message level
Expand All @@ -57,4 +91,17 @@ export class Annotations {
private addMessage(level: string, message: string) {
this.scope.node.addMetadata(level, message);
}
}

/**
* 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 });
}

return set;
}
}
24 changes: 13 additions & 11 deletions packages/@aws-cdk/core/lib/construct-compat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,6 @@ export class Construct extends constructs.Construct implements IConstruct {
return typeof x === 'object' && x !== null && CONSTRUCT_SYMBOL in x;
}

/**
* The construct tree node associated with this construct.
*
* @deprecate `Construct.node` is being deprecated in favor of
* `Construct.construct`. This API will be removed in the next major version
* of the AWS CDK, please migrate your code to use `construct` instead.
*/
public readonly node: ConstructNode;

/**
* Construct API.
*/
Expand All @@ -91,8 +82,7 @@ export class Construct extends constructs.Construct implements IConstruct {
}

Object.defineProperty(this, CONSTRUCT_SYMBOL, { value: true });
this.node = ConstructNode._unwrap(constructs.Node.of(this));
this.construct = this.node;
this.construct = ConstructNode._unwrap(constructs.Node.of(this));

const disableTrace =
this.construct.tryGetContext(cxapi.DISABLE_METADATA_STACK_TRACE) ||
Expand All @@ -106,6 +96,18 @@ export class Construct extends constructs.Construct implements IConstruct {
}
}

/**
* The construct tree node associated with this construct.
*
* @deprecate `Construct.node` is being deprecated in favor of
* `Construct.construct`. This API will be removed in the next major version
* of the AWS CDK, please migrate your code to use `construct` instead.
*/
public get node(): ConstructNode {
Annotations.of(this).addDeprecation('@aws-cdk/core.Construct.node', 'Use "@aws-cdk/core.Construct.construct" instead');
return this.construct;
}

/**
* Validate the current construct.
*
Expand Down
95 changes: 95 additions & 0 deletions packages/@aws-cdk/core/test/test.annotations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { CloudAssembly } from '@aws-cdk/cx-api';
import { Test } from 'nodeunit';
import { Construct, App, Stack } from '../lib';
import { Annotations } from '../lib/annotations';

const restore = process.env.CDK_BLOCK_DEPRECATIONS;

export = {
'tearDown'(cb: any) {
process.env.CDK_BLOCK_DEPRECATIONS = restore; // restore to the original value
cb();
},

'addDeprecation() annotates the usage of a deprecated API'(test: Test) {
// GIVEN
const app = new App();
const stack = new Stack(app, 'MyStack');
const c1 = new Construct(stack, 'Hello');

// WHEN
delete process.env.CDK_BLOCK_DEPRECATIONS;
Annotations.of(c1).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');

// THEN
test.deepEqual(getWarnings(app.synth()), [
{
path: '/MyStack/Hello',
message: 'The API @aws-cdk/core.Construct.node is deprecated: use @aws-cdk.Construct.construct instead. This API should will be removed in the next major version of the AWS CDK',
},
]);
test.done();
},

'deduplicated per node based on "api"'(test: Test) {
// GIVEN
const app = new App();
const stack1 = new Stack(app, 'MyStack1');
const stack2 = new Stack(app, 'MyStack2');
const c1 = new Construct(stack1, 'Hello');
const c2 = new Construct(stack1, 'World');
const c3 = new Construct(stack2, 'FooBar');

// WHEN
delete process.env.CDK_BLOCK_DEPRECATIONS;
Annotations.of(c1).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');
Annotations.of(c2).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');
Annotations.of(c1).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');
Annotations.of(c3).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');
Annotations.of(c1).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');
Annotations.of(c1).addDeprecation('@aws-cdk/core.Construct.node', 'use @aws-cdk.Construct.construct instead');

// THEN
test.deepEqual(getWarnings(app.synth()), [
{
path: '/MyStack1/Hello',
message: 'The API @aws-cdk/core.Construct.node is deprecated: use @aws-cdk.Construct.construct instead. This API should will be removed in the next major version of the AWS CDK',
},
{
path: '/MyStack1/World',
message: 'The API @aws-cdk/core.Construct.node is deprecated: use @aws-cdk.Construct.construct instead. This API should will be removed in the next major version of the AWS CDK',
},
{
path: '/MyStack2/FooBar',
message: 'The API @aws-cdk/core.Construct.node is deprecated: use @aws-cdk.Construct.construct instead. This API should will be removed in the next major version of the AWS CDK',
},
]);
test.done();
},

'CDK_BLOCK_DEPRECATIONS will throw if a deprecated API is used'(test: Test) {
// GIVEN
const app = new App();
const stack = new Stack(app, 'MyStack');
const c1 = new Construct(stack, 'Hello');

// THEN
process.env.CDK_BLOCK_DEPRECATIONS = '1';
test.throws(() => Annotations.of(c1).addDeprecation('foo', 'bar'), /MyStack\/Hello: The API foo is deprecated: bar\. This API should will be removed in the next major version of the AWS CDK/);
test.done();
},
};

function getWarnings(casm: CloudAssembly) {
const result = new Array<{ path: string, message: string }>();
for (const stack of Object.values(casm.manifest.artifacts ?? {})) {
for (const [path, md] of Object.entries(stack.metadata ?? {})) {
for (const x of md) {
if (x.type === 'aws:cdk:warning') {
result.push({ path, message: x.data as string });
}
}
}
}
return result;
}
3 changes: 2 additions & 1 deletion tools/cdk-build-tools/bin/cdk-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ async function main() {
const defaultShellOptions = {
timers,
env: {
CDK_DISABLE_STACK_TRACE: '1',
CDK_DISABLE_STACK_TRACE: '1', // skip all stack trace collection (expensive)
CDK_BLOCK_DEPRECATIONS: '1', // forbid the usage of deprecated APIs
},
};

Expand Down

0 comments on commit 006954d

Please sign in to comment.