Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(cli): watch command not working properly with NodeJsFunction #17391

Closed
tmokmss opened this issue Nov 8, 2021 · 1 comment · Fixed by #17455
Closed

(cli): watch command not working properly with NodeJsFunction #17391

tmokmss opened this issue Nov 8, 2021 · 1 comment · Fixed by #17455
Assignees
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 package/tools Related to AWS CDK Tools or CLI

Comments

@tmokmss
Copy link
Contributor

tmokmss commented Nov 8, 2021

What is the problem?

Hi, I tried new cdk watch command but it's not working as expected. Here's the detail:

When we cdk watch a stack with a NodeJsFunction, the resulting asset of Lambda code becomes broken.
Note that cdk deploy --watch seems working fine, so it isn't a big issue.

Reproduction Steps

cdk watch a stack with NodeJsFunction, and then modify lambdas/handler/index.ts to trigger a new deploy.

import * as cdk from "@aws-cdk/core";
import * as lambda from "@aws-cdk/aws-lambda-nodejs";

export class TestStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);
    const handler = new lambda.NodejsFunction(this, "Handler", {
      entry: "lambdas/handler/index.ts"
    });
  }
}

in lambdas/handler/ directory, there is only a file index.ts

export const handler = async () => {
  return ""
}

What did you expect to happen?

The resulting asset contains only files related to the Lambda function such as index.js.

What actually happened?

The resulting asset contains all the files in the CDK root directory.

CDK CLI Version

1.131.0

Framework Version

1.131.0

Node.js Version

v14.15.0

OS

macOS

Language

Typescript

Language Version

No response

Other information

I found why it's happening as below:

  1. When synthesizing assets, bundling code is skipped because skip = true is passed. When skipped, the resulting asset would become entire CDK root directory in my case.

    if (skip) {
    // We should have bundled, but didn't to save time. Still pretend to have a hash.
    // If the asset uses OUTPUT or BUNDLE, we use a CUSTOM hash to avoid fingerprinting
    // a potentially very large source directory. Other hash types are kept the same.
    let hashType = this.hashType;
    if (hashType === AssetHashType.OUTPUT || hashType === AssetHashType.BUNDLE) {
    this.customSourceFingerprint = Names.uniqueId(this);
    hashType = AssetHashType.CUSTOM;
    }
    return {
    assetHash: this.calculateHash(hashType, bundling),
    stagedPath: this.sourcePath,
    packaging: FileAssetPackaging.ZIP_DIRECTORY,
    isArchive: true,
    };
    }

  2. the skip flag becomes true because this.node.tryGetContext(cxapi.BUNDLING_STACKS) is []

    if (props.bundling) {
    // Check if we actually have to bundle for this stack
    const bundlingStacks: string[] = this.node.tryGetContext(cxapi.BUNDLING_STACKS) ?? ['*'];
    skip = !bundlingStacks.find(pattern => minimatch(Stack.of(this).stackName, pattern));
    const bundling = props.bundling;
    stageThisAsset = () => this.stageByBundling(bundling, skip);

  3. this.node.tryGetContext(cxapi.BUNDLING_STACKS) is [] because BUNDLING_COMMANDS does not include watch command.

    if (BUNDLING_COMMANDS.includes(argv._[0])) {
    // If we deploy, diff or synth a list of stacks exclusively we skip
    // bundling for all other stacks.
    bundlingStacks = argv.exclusively
    ? argv.STACKS ?? ['*']
    : ['*'];
    } else { // Skip bundling for all stacks
    bundlingStacks = [];
    }

    const BUNDLING_COMMANDS = [
    Command.DEPLOY,
    Command.DIFF,
    Command.SYNTH,
    Command.SYNTHESIZE,
    ];

So I guess we can solve this issue by something like adding watch to BUNDLING_COMMANDS array. (I don't know if there'd be some side effects though...)

btw it worked well with cdk deploy --watch command, thanks for a great new feature!

@tmokmss tmokmss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 8, 2021
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Nov 8, 2021
@ryparker ryparker added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Nov 8, 2021
jogold added a commit to jogold/aws-cdk that referenced this issue Nov 10, 2021
Add `watch` to the list of commands that require bundling.

Closes aws#17391
jogold added a commit to jogold/aws-cdk that referenced this issue Nov 10, 2021
Add `watch` to the list of commands that require bundling.

Closes aws#17391
@mergify mergify bot closed this as completed in #17455 Nov 10, 2021
mergify bot pushed a commit that referenced this issue Nov 10, 2021
Add `watch` to the list of commands that require bundling.

Closes #17391


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
Add `watch` to the list of commands that require bundling.

Closes aws#17391


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants