Skip to content

Commit

Permalink
chore(core): isCfnResource allows any type as input (aws#28001)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
tmokmss authored and chenjane-dev committed Dec 5, 2023
1 parent 670fb61 commit 4595c86
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/core/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions packages/aws-cdk-lib/core/test/cfn-resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

0 comments on commit 4595c86

Please sign in to comment.