From e21916d99b266640fd81a0488f5cff3e2018acf9 Mon Sep 17 00:00:00 2001 From: Masashi Tomooka Date: Wed, 29 Nov 2023 11:20:23 +0900 Subject: [PATCH] chore(core): isCfnResource allows any type as input (#28001) It is more or less frustrating that we have to check if a variable is undefined or not before calling the `CfnResource.isCfnResource` method. For example, ```ts const bucket1 = new Bucket(stack, 'Bucket1'); const bucket1Resource = bucket1.node.defaultChild; if (bucket1Resource !== undefined && // Currently we need this! cdk.CfnResource.isCfnResource(bucket1Resource) ) { bucket1Resource.addDependency(...); } ``` With this PR, `isCfnResource` now accepts `any` type as input and performs the necessary validations inside. ```ts const bucket1 = new Bucket(stack, 'Bucket1'); const bucket1Resource = bucket1.node.defaultChild; if (cdk.CfnResource.isCfnResource(bucket1Resource)) { // much smoother bucket1Resource.addDependency(...); } ``` Actually, other `isXxx` methods have consistent signatures like the one below: ```ts public static isStack(x: any): x is Stack public static isReference(x: any): x is Reference public static isCfnElement(x: any): x is CfnElement // and more... ``` This change also makes the `isCfnResource` consistent with these signatures. Note that this is not a breaking change, because the input constraint is relaxed, not tightened, so all the old code will work without change. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk-lib/core/lib/cfn-resource.ts | 8 +++---- .../core/test/cfn-resource.test.ts | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/core/lib/cfn-resource.ts b/packages/aws-cdk-lib/core/lib/cfn-resource.ts index c6c069f27e71f..a4dfd3066aaf4 100644 --- a/packages/aws-cdk-lib/core/lib/cfn-resource.ts +++ b/packages/aws-cdk-lib/core/lib/cfn-resource.ts @@ -4,7 +4,7 @@ import { CfnCondition } from './cfn-condition'; /* eslint-disable import/order */ import { CfnRefElement } from './cfn-element'; import { CfnCreationPolicy, CfnDeletionPolicy, CfnUpdatePolicy } from './cfn-resource-policy'; -import { Construct, IConstruct, Node } from 'constructs'; +import { Construct, Node } from 'constructs'; import { addDependency, obtainDependencies, removeDependency } from './deps'; import { CfnReference } from './private/cfn-reference'; import { Reference } from './reference'; @@ -34,10 +34,10 @@ export interface CfnResourceProps { */ export class CfnResource extends CfnRefElement { /** - * Check whether the given construct is a CfnResource + * Check whether the given object is a CfnResource */ - public static isCfnResource(construct: IConstruct): construct is CfnResource { - return (construct as any).cfnResourceType !== undefined; + public static isCfnResource(x: any): x is CfnResource { + return x !== null && typeof(x) === 'object' && x.cfnResourceType !== undefined; } // MAINTAINERS NOTE: this class serves as the base class for the generated L1 diff --git a/packages/aws-cdk-lib/core/test/cfn-resource.test.ts b/packages/aws-cdk-lib/core/test/cfn-resource.test.ts index d8c8fab44d5b5..78230863d3f51 100644 --- a/packages/aws-cdk-lib/core/test/cfn-resource.test.ts +++ b/packages/aws-cdk-lib/core/test/cfn-resource.test.ts @@ -412,4 +412,28 @@ describe('cfn resource', () => { delete process.env.CDK_DEBUG; } }); + + test('isCfnResource returns true with a CfnResource', () => { + const app = new core.App(); + const stack = new core.Stack(app, 'Stack'); + const res = new core.CfnResource(stack, 'SomeCfnResource', { + type: 'Some::Resource', + }); + + // THEN + expect(core.CfnResource.isCfnResource(res)).toBe(true); + }); + + test('isCfnResource returns false with a construct', () => { + const app = new core.App(); + const stack = new core.Stack(app, 'Stack'); + + // THEN + expect(core.CfnResource.isCfnResource(stack)).toBe(false); + }); + + test('isCfnResource returns false with undefined', () => { + // THEN + expect(core.CfnResource.isCfnResource(undefined)).toBe(false); + }); });