[core] AssetStaging by bundling ignoring AssetHashType.OUTPUT when doing quick 'skip' hash

[mrgrain]

AssetStaging provides a shortcut when using custom bundling interface which allows to skip the normal hashing method. Instead it falls back to AssetHashType.SOURCE with a comment implying it would be quicker. This code path seems to be only invoked when cdk list is called, but I might have misunderstood the condition.

Code Reference:

aws-cdk/packages/@aws-cdk/core/lib/asset-staging.ts

Lines 240 to 248 in 21ccfce

	private stageByBundling(bundling: BundlingOptions, skip: boolean): StagedAsset {
	if (skip) {
	// We should have bundled, but didn't to save time. Still pretend to have a hash,
	// but always base it on sources.
	return {
	assetHash: this.calculateHash(AssetHashType.SOURCE),
	stagedPath: this.sourcePath,
	};
	}

This shortcut is also taken when assetHashType set to OUTPUT. Obviously this won't be calculating the correct hash, but I'm not sure what the implication of a different hash is. In any case, I believe the assumption that SOURCE hashing is always faster than OUTPUT is a wrong assumption.

Reproduction Steps

Apologies for not providing a code sample at this point. But it can be recreated with a simple setup:

• Add a S3.Asset to a stack pointing to a large folder (e.g. one that contains a lot of node_modules)
• Provide a bundling method that can do bundling really really fast (e.g. esbuild or a fake one)
• Run time cdk synth, then run time cdk ls

What did you expect to happen?

cdk ls should be quicker or at leat taking the same time as cdk synth.

What actually happened?

cdk ls takes significantly longer than cdk synth, since calculating the assetHash from a large source takes much longer than running a very quick bundler and hashing a very small number of output files.

Environment

• CDK CLI Version: 1.73.0
• Framework Version: 1.73.0
• Node.js Version: v12.18.0
• OS : macOS 10.15.7 (19H2)
• Language (Version): TypeScript 4.0.5

Other

I'm happy to work on this issue, it seems very straight forward enough.

However, combined with its sibling issue #11459 this would just result in removing the skip part of stageByBundling. I'm wondering if we could just remove random or empty data instead, but I'm not sure if cdk ls will rely on any of it.

---

This is 🐛 Bug Report

[jogold]

You are correct, see #11440

[mrgrain]

Ah sorry! I had checked issues and PRs yesterday while investigating, but not again today. 🤦

I was actually wondering if you run into the same issue 😄

[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.