External libraries in v0.33.0 throwing ${instance} does not implement DependableTrait

[joshlartz]

Describe the bug
I've got a few import helper functions I'm pulling in from a cdk library and getting some unexpected behavior trying to make it work on v0.33. If I move the functions into the same lib/stack.ts file they're being called from it works as expected.

From debugging, this error is thrown after the stack is processed.

To Reproduce

app lib/stack.ts

import cdk = require('@aws-cdk/cdk');
import ecs = require('@aws-cdk/aws-ecs');
import ecsL3 = require('@aws-cdk/aws-ecs-patterns')
import ec2 = require('@aws-cdk/aws-ec2');
import cdkLib = require('@cdkLib/cdk');

export class CdkEcsStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const cluster = cdkLib.clusterImport(this, 'Cluster');

    const service = new ecsL3.LoadBalancedEc2Service(this, 'Service', {
      cluster: cluster,
      memoryLimitMiB: 1024,
      image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
      desiredCount: 2,
    });
  }
}

library lib/context-providers/vpcs.ts

import cdk = require('@aws-cdk/cdk');
import ec2 = require('@aws-cdk/aws-ec2');

const Account_1_US_EAST_1 = {
  vpcId: 'vpc-12345',
  availabilityZones: ['us-east-1a', 'us-east-1b'],
  publicSubnetIds: ['subnet-12345', 'subnet-67890'],
  privateSubnetIds: ['subnet-12345', 'subnet-67890']
}
export function vpcImport(scope: cdk.Construct, name: string) {
  const account = scope.node.stack.env.account;
  const region = scope.node.stack.env.region;

  if (account == '1234567890' && region == 'us-east-1') {
    return ec2.Vpc.fromVpcAttributes(scope, name, Account_1_US_EAST_1);
    } else {
    throw new Error(`VPC not found in account ${account}, region ${region}`)
  }
}

library lib/context-providers/ecs.ts

import cdk = require('@aws-cdk/cdk');
import ecs = require('@aws-cdk/aws-ecs');
import ec2 = require('@aws-cdk/aws-ec2');
import { vpcImport } from './vpcs';

export function clusterImport(scope: cdk.Construct, name: string) {
  const account = scope.node.stack.env.account;
  const region = scope.node.stack.env.region;
  const vpc = vpcImport(scope, 'Vpc');

  if (account == '1234567890' && region == 'us-east-1') {
    return ecs.Cluster.fromClusterAttributes(scope, name, {
      clusterName: 'name',
      securityGroups: [ec2.SecurityGroup.fromSecurityGroupId(scope, 'ecs', 'sg-123456')],
      vpc: vpc
    });
  } else {
    throw new Error(`Cluster not found in account ${account}, region ${region}`);
  }
}

cdk ls or diff

Expected behavior
The external functions should resolve like normal and allow the stack to be built and diff/ls commands to work.

Version:

• MacOS 10.14.5
• TS 3.3.333 ts-node 8.2.0 Node 12.3.1
• CDK Version 0.33.0

[simalexan]

Confirming this issue.

[sam-goodwin]

DependableTrait uses a private static WeakMap to lookup an object's implementation. This works fine unless you are instantiating a Construct from a dependent module that doesn't share the same @aws-cdk/cdk module.

https://github.com/awslabs/aws-cdk/blob/696f53f8ee426a7a7958d0807c7ad89b72f587ae/packages/%40aws-cdk/cdk/lib/dependency.ts#L61-L94

This will affect any L3 framework/library since they will have a nested dependency on @aws-cdk/cdk.

Instead of using a WeakMap to track an object's implementation, how about we store the trait on the object with an explicitly named Symbol?

// change implement to:
public static implement(instance: IDependable, trait: DependableTrait) {
  (instance as any)[Symbol.for('@aws-cdk/cdk/DependableTrait')] = trait;
}

// change get to:
public static get(instance: IDependable): DependableTrait {
  const ret = (instance as any)[Symbol.for('@aws-cdk/cdk/DependableTrait')];
  if (!ret) {
    throw new Error(`${instance} does not implement DependableTrait`);
  }
  return ret;
}

[joshlartz]

The issue is still present in v0.34.0 too.

[auser]

I can confirm it is still an issue in 0.34.0 as well.

[archikierstead]

yes same here, still present in 0.34.0

[skinny85]

Out of curiosity @joshlartz - in your cdk library with the helper functions, how are you defining your dependency on the cdk packages? Through the dependencies field in package.json, or through peerDependencies?

[joshlartz]

@skinny85 All the cdk packages are regular dependencies with @aws-cdk/cdk additionally being a peerDependency. I believe that is the standard way the library is created with cdk init.

[orangewise]

@rix0rrr, what do you think of the suggestion of @sam-goodwin? I'm kinda stuck at 0.29.0 now :(

[rix0rrr]

I'm not sure how we end up in the situation where we have 2 different copies of the CDK core library.

I thought our use of peerDependencies was supposed to (help) prevent that.