Skip to content

Commit

Permalink
fix(core): Make filterUndefined null-safe (#2789)
Browse files Browse the repository at this point in the history
There are a couple of places where fields accept values that are typed
as `Json` per the JSII type specification. This conveys that literal
`null` values may be passed and need to be preserved (as far as JSII is
concerned - see aws/jsii#523). However in the CloudFormation domain,
`null` is semantically equivalent to `undefined`.

Now enters Javascript's confusing type system, where `null` is an
`object` that cannot be converted to `object` (you read this correctly):

```js
typeof null === 'object'
// => true

Object.entries(null);
// => Thrown:
//    TypeError: Cannot convert undefined or null to object
//        at Function.entries (<canonymous>)
```

So this changes the `undefined` checks to the `null`-coercing way, so
that `null` and `undefined` are handled the same way.
  • Loading branch information
RomainMuller authored and rix0rrr committed Jun 7, 2019
1 parent 5c90256 commit e4fb811
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/cdk/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ export function ignoreEmpty(obj: any): any {
}

/**
* Returns a copy of `obj` without undefined values in maps or arrays.
* Returns a copy of `obj` without `undefined` (or `null`) values in maps or arrays.
*/
export function filterUndefined(obj: any): any {
if (Array.isArray(obj)) {
return obj.filter(x => x !== undefined).map(x => filterUndefined(x));
return obj.filter(x => x != null).map(x => filterUndefined(x));
}

if (typeof(obj) === 'object') {
const ret: any = { };
for (const [key, value] of Object.entries(obj)) {
if (value === undefined) {
if (value == null) {
continue;
}
ret[key] = filterUndefined(value);
Expand Down
20 changes: 16 additions & 4 deletions packages/@aws-cdk/cdk/test/test.util.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Test } from 'nodeunit';
import { Test, testCase } from 'nodeunit';
import { Stack } from '../lib';
import { capitalizePropertyNames, ignoreEmpty } from '../lib/util';
import { capitalizePropertyNames, filterUndefined, ignoreEmpty } from '../lib/util';

export = {
export = testCase({
'capitalizeResourceProperties capitalizes all keys of an object (recursively) from camelCase to PascalCase'(test: Test) {
const c = new Stack();

Expand Down Expand Up @@ -71,8 +71,20 @@ export = {
test.deepEqual(stack.resolve(ignoreEmpty({ xoo: { resolve: () => [ undefined, undefined ] }})), { xoo: [] });
test.done();
}
},

'filterUnderined': {
'is null-safe (aka treats null and undefined the same)'(test: Test) {
test.deepEqual(filterUndefined({ 'a null': null, 'a not null': true }), { 'a not null': true });
test.done();
},

'removes undefined, but leaves the rest'(test: Test) {
test.deepEqual(filterUndefined({ 'an undefined': undefined, 'yes': true }), { yes: true });
test.done();
}
}
};
});

class SomeToken {
public foo = 60;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ import { ConstructNode, Stack } from '../lib';

export function toCloudFormation(stack: Stack): any {
return ConstructNode.synth(stack.node, { skipValidation: true }).getStack(stack.name).template;
}
}

0 comments on commit e4fb811

Please sign in to comment.