(aws-lambda-nodejs): ICommandHooks requires all hooks to be implementated

[mjwbenton]

Reproduction Steps

    new lambda.NodejsFunction(this, "LambdaFunction", {
      bundling: {
        commandHooks: {
          afterBundling(inputDir: string, outputDir: string): string[] {
            return [`echo "hello world"`];
          },
        },
      },
    });

What did you expect to happen?

For this to be valid, and the equivalent of defining no commands for beforeBundling and beforeInstall.

What actually happened?

Type '{ afterBundling(inputDir: string, outputDir: string): string[]; }' is missing the following properties from type 'ICommandHooks': beforeBundling, beforeInstall

Environment

• CDK CLI Version : 1.91.0
• Framework Version: 1.91.0
• Node.js Version: v14.15.1
• OS : macOS Catalina
• Language (Version): Typescript (4.2.3)

Other

Happy to provide a pull-request for this, looks very easy to fix. Unless I'm missing something we can just declare the functions as optional and then default them to the equivalent of () => [].

---

This is :bug: Bug Report

[jogold]

@eladb this is https://github.com/aws/aws-cdk/pull/11583#discussion_r528930622

what do you thinK?

[mjwbenton]

Ah yeah, it looks like they were made non-optional for jsii reasons then. I did wonder if that was what I was missing.

It's pretty strange from a user perspective, but I understand not wanting the experience to be different across different languages. It's not super important, so fair enough if you want to do nothing, but I would have been less likely to question it if maybe the example in the documentation showed...

  beforeBundling: () => []
  beforeInstall: () => []

rather than // ...

I'm not familiar with the ins and outs of JSII, and I'm only deeply familiar with Java when it comes to the other languages, but trying to think about ways around this.

If you were building this out natively in Java you'd likely define a functional interface for a hook function, something like...

interface CommandHook {
  List<String> hook(string inputDir, string outputDir);
}

Then you'd use lambdas that adhere to that interface as values, so that null becomes a possibility and therefore their implementation is optional. I'm guessing going down that path isn't possible with JSII? Are there other examples in the CDK where functions are passed as values?

The only other thing that I can think, that I assume must be compatible with JSII, is to wrap each function up in another object so that the whole of that can be optional in the way that commandHooks is optional today.

interface CommandHookFunction {
  run: (inputDir: string, outputDir: string) => string[]
}

interface CommandHooks {
  beforeBundling?: CommandHookFunction,
  afterBundling?: CommandHookFunction,
  beforeInstall?: CommandHookFunction
}

commandHooks: {
  beforeBundling: {
    run: (inputDir: string, outputDir: string) => [...]
  }
}

[eladb]

Yeah it's a limitation we have. We could split up the interface to multiple interfaces (one for each hook), and then make the props optional

[eladb]

I am unassigning and marking this issue as p2 which means that we are unable to work on this immediately.

We use +1s to help us prioritize our work, and as always we are happy to take contributions if anyone is interested to pick this up and submit a PR (please make sure to follow our contribution guidelines.

🙏

[piotrekwitkowski]

A Python example how to use command_hooks would be greatly appreciated!

[icj217]

Would also appreciate a python example. I've tried a couple different permutations but all of them result in TypeError: Don't know how to convert object to JSON: <class 'helpers.bundling.CustomCommandHooks'> errors

Attempts:

from aws_cdk import (
    aws_lambda_nodejs as njs,
)
class CustomCommandHooks(njs.ICommandHooks):
    def before_bundling(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return []

as well as:

@jsii.interface(jsii_type="aws-cdk-lib.aws_lambda_nodejs.ICommandHooks")
class CustomCommandHooks:

    @jsii.member(jsii_name="afterBundling")
    def after_bundling(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return []

    @jsii.member(jsii_name="beforeBundling")
    def before_bundling(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return [f"cd {input_dir}", "npm install"]

    @jsii.member(jsii_name="beforeInstall")
    def before_install(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return []

Commenting out the @jsii.member method decorators yields same error.

Changing the class decorator to @jsii.implements(njs.ICommandHooks) in previous example yields AttributeError: type object 'type' has no attribute '__jsii_type__' error.

[icj217]

Was able to figure out how to implement this in Python. The key step was creating an instance of the class and passing that into BundlingOptions

import typing
import builtins
import jsii
from aws_cdk import (
    aws_lambda_nodejs as njs,
)
​
@jsii.implements(njs.ICommandHooks)
class CustomCommandHooks:
​
    @jsii.member(jsii_name="afterBundling")
    def after_bundling(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return []
​
    @jsii.member(jsii_name="beforeBundling")
    def before_bundling(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return [f"cd {input_dir}", "yarn"]
​
    @jsii.member(jsii_name="beforeInstall")
    def before_install(self, input_dir: builtins.str, output_dir: builtins.str) -> typing.List[builtins.str]:
        return []

...

hooks = CustomCommandHooks()
​
options = njs.BundlingOptions(
        source_map=True,
        target="es2020",
        format=njs.OutputFormat.CJS,
        loader={".yaml": "file"},
        command_hooks=hooks)

@piotrekwitkowski FYI

[madeline-k]

There is also a basic example in the README for this module: https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_lambda_nodejs/README.html#command-hooks.