[core] Multiple projects breaks CDK synth (using instanceof cross-project)

[tobias-bardino]

❓ General Issue

Creating a Construct, shared by multiple stacks, in a separate project, fails to deploy in CDK version >= 1.57.0

In our setup we have multiple stacks sharing misc. Constructs. These Constructs are all in separate Typescript/NodeJS projects.
We use Lerna to maintain/link dependencies between projects.

A simplified example of the structure could be:

➜ tree
.
├── README.md
├── lerna.json
├── package-lock.json
├── package.json
└── packages
    ├── myapp
    │   ├── README.md
    │   ├── bin
    │   │   └── myapp.ts
    │   ├── cdk.json
    │   ├── jest.config.js
    │   ├── lambda
    │   │   └── index.ts
    │   ├── lib
    │   │   └── myapp-stack.ts
    │   ├── package-lock.json
    │   ├── package.json
    │   ├── test
    │   │   └── myapp.test.ts
    │   └── tsconfig.json
    └── myconstruct
        ├── README.md
        ├── jest.config.js
        ├── lambda
        │   └── index.ts
        ├── lib
        │   └── index.ts
        ├── package-lock.json
        ├── package.json
        ├── test
        │   └── myconstruct.test.ts
        └── tsconfig.json

(an example can be found here: https://github.com/gwriss/upgrade_bug)

Running

➜ npx lerna bootstrap

will npm-install all dependencies and link projects like this (snipped for better overview):

➜ tree -L 4
.
├── ...
└── packages
    ├── myapp
    │   ├── ...
    │   ├── node_modules
    │   │   ├── ...
    │   │   ├── @aws-cdk
    │   │   ├── ...
    │   │   ├── myconstruct -> ../../myconstruct
    │   │   └── ...
    │   └── ...
    └── myconstruct
        ├── ...
        ├── node_modules
        │   ├── ...
        │   ├── @aws-cdk
        │   └── ...
        └── ...

Now, when the Construct in myconstruct contains a Lambda Function and the Myapp in myapp instantiate an instance of Myconstruct, the synth of the app fails:

➜ npx lerna run build
lerna notice cli v3.22.1
...
...
lerna success - myapp
lerna success - myconstruct
➜ cd packages/myapp
➜ npx cdk synth
unable to determine cloud assembly output directory. Assets must be defined indirectly within a "Stage" or an "App" scope
Subprocess exited with error 1

(an example of the setup can be found and tested here: https://github.com/gwriss/upgrade_bug)

Diving into the problem, it seems like it's another issue with instanceof comparing two instances of the same class, but from different node_modules.

I think @tneely was onto something in #9546 (comment), but decided to close the issue without a resolution.

Another issued related to this is the "Duck Typing" fix of Stack in #10671.

⚠️ Notice
Please don't pollute the discussion with suggestions about "deleting node_modules & reinstall" and "checking modules are all of the same version".
That is another issue but has nothing to do with this issue!

The Question

Did I miss som crucial point in our preferred project setup? I would really like a discussion about the recommended way of organising bigger CDK apps/projects. This could hopefully lead to a new topic in developer guide.

@rix0rrr suggest a peerDependency solution for a similar problem here: 5e10b94
but I fail to see how this would solve the problem. (please enlighten me if I missed something 😃 )

At the moment this is a show-stopper for our long-overdue upgrade of CDK

Reference

• [core] Unable to Synthesize CDK in 1.57: Assets must be defined indirectly within a "Stage" or an "App" scope #9546
• [core] SynthUtils.toCloudFormation(stack) fails on stacks defined in external project  #10671
• https://github.com/gwriss/upgrade_bug

Environment

• CDK CLI Version: 1.68.0 (build a6a3f46)
• Module Version:
• Node.js Version: v12.19.0
• OS:
• Language (Version): TypeScript 3.9.7

Other information

[tobias-bardino]

BTW. I tried to fix this issue by replacing instanceof with "Duck Typing" in stage.ts

diff --git a/packages/@aws-cdk/core/lib/stage.ts b/packages/@aws-cdk/core/lib/stage.ts
index 50ef0b7c5..7e110ba40 100644
--- a/packages/@aws-cdk/core/lib/stage.ts
+++ b/packages/@aws-cdk/core/lib/stage.ts
@@ -7,6 +7,8 @@ import { synthesize } from './private/synthesis';
 // eslint-disable-next-line
 import { Construct as CoreConstruct } from './construct-compat';

+const STAGE_SYMBOL = Symbol.for('@aws-cdk/core.Stage');
+
 /**
  * Initialization props for a stage.
  */
@@ -84,8 +86,12 @@ export class Stage extends CoreConstruct {
    *
    * @experimental
    */
-  public static isStage(x: any ): x is Stage {
-    return x !== null && x instanceof Stage;
+  public static isStage(x: any): x is Stage {
+    // Original check
+    // return x !== null && x instanceof Stage;
+
+    // Duck typing check copied from stack.ts:153 isStack(x: any)
+    return x !== null && typeof (x) === 'object' && STAGE_SYMBOL in x;
   }

   /**

But that kind of blew up the tests in core

Test result after duck typing fix

@aws-cdk/core: Test Suites: 35 failed, 14 passed, 49 total
@aws-cdk/core: Tests:       300 failed, 249 passed, 549 total
@aws-cdk/core: Snapshots:   0 total
@aws-cdk/core: Time:        10.622 s
@aws-cdk/core: Ran all test suites.

Maybe someone with more in-depth experience with the core package can see what is going on.

[tobias-bardino]

This seems to be the same problem but the issue is closed: #10210 (comment)

[rix0rrr]

I think this just flat-out won't work, and Lerna isn't smart enough to do it.

Yarn will hoist the modules appropriately. Otherwise you're going to have to declare a dependency on CDK in devDependencies at the top-level of your monorepo.

[tobias-bardino]

I understand that CDK, as a project, does not want to fix the dependency problems with npm/lerna. Totally understandable.

Still, this will most likely become a problem lots of people will face in the future, when their CDK projects gets bigger and they need to rearrange their source code into separate projects.

@rix0rrr would it be interesting to have a section in the developer guide for organising bigger CDK projects? What is the best way to proceed with this?

[tobias-bardino]

#11113 will most likely fix this issue (still needs to be confirmed when CDK 1.72.0 is released)

[tobias-bardino]

I think this just flat-out won't work, and Lerna isn't smart enough to do it.

Yarn will hoist the modules appropriately. Otherwise you're going to have to declare a dependency on CDK in devDependencies at the top-level of your monorepo.

For future reference, when people face this issue:

I created a new branch use_lerna_yarn_workspaces testing @rix0rrr suggestion using yarn hoisting.

It works... but please read this before choosing this path: https://rushjs.io/pages/advanced/phantom_deps/

For our project we chose another path, using Rush instead of lerna/yarn. The switch is not a not small task for a big project, but the dependency handling solutions seems so much better, we figured it was worth it. See this branch: try_with_rush for a minimal example of what is required and that it actually works.

Note ⚠️
Rush is very opinionated but will solve the dependency problems we are facing.
For fixing all dependency problems (read: https://rushjs.io/pages/advanced/npm_doppelgangers/) we also need to switch to pnpm, which is a even bigger task, that probably will introduce new problems (https://rushjs.io/pages/maintainer/package_managers/). We postponed that for now, and are still using npm.

I will close this issue because I don't expect any solution to be found within the CDK project.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.