Aws-cdk: lambda-nodejs to support custom Builder implementations

[mrgrain]

The aws-lambda-nodejs module abstracts the package building into a Builder class. The package should support providing custom implementations for this class so other bundlers than parcel can be used, and more complex use cases are supported.

Use Case

• The Builder has been converted to use Docker to solve certain issues, however using Docker might create issues in other areas (CI often runs in containers and Docker-in-Docker configurations can be complex)
• Using other bundlers like zeit/ncc or webpack
• Easier support for complex parcel configurations/plugins
• Support for other bundling flows, like zip https://github.com/aws/aws-cdk/issues/6294

Proposed Solution

Keep the original Builder and BuilderOptions interface pre Docker

export interface BuilderOptions {
  readonly entry: string;
  readonly outDir: string;
  readonly global: string;
  readonly minify?: boolean;
  readonly sourceMaps?: boolean;
  readonly cacheDir?: string;
  readonly nodeVersion?: string;
}

Every Builder must implement the interface, but may extend the options and also might choose to discard the optional options for implementation.

Extend NodejsFunctionProps to include

export interface NodejsFunctionProps extends lambda.FunctionOptions {
  readonly builderFactory?: (props: BuilderOptions) => Builder;
  readonly additionalBuilderProps?: object;
}

builderFactory would receive a combination of mandatory and additional builder props:

builderFactory({...additionalBuilderProps, mandatoryBuilderProps})

If no builderFactory is provided, the class would fall back to a default implementation using parcel in docker.

Other

The Builder class interface seems to be stable enough. It survived the migration to Docker with few changes.

I'm not sure about the Design, especially if there's something similar used in CDK elsewhere or a more suitable implementation.

Further changes to the Builder might make that class more extendable (e.g. the error handling).

Alternatives

The Builder factory could become a static method on the NodejsFunction class so extending the class might provide an alternative option (albeit less first class support).

class MyFunctionBundlerFunction extends lNodejsFunction {
  public static function getBuilder(props: BuilderOptions) {
    return new new Builder({
      ...props,
      someOtherOption: 'a-value',
    });
  }
}

• [x] :wave: I may be able to implement this feature request
• [x] :warning: This feature might incur a breaking change --> Maybe. I think it could be done either way. In order to support a better, future proof design a breaking change might be required.

---

This is a :rocket: Feature Request

[mrgrain]

@eladb I'd be happy to create a PR for this, would like some input/confirmation on the interfaces.

[jogold]

The Builder class interface seems to be stable enough. It survived the migration to Docker with few changes.

We reverted the Docker migration for the moment (#7738) but I'm close to finding a fix for this. PR to come soon. Then we can maybe think about "generalizing" the Docker build concept for a Lambda function (I've been looking into this and it's not easy to cover all cases).

For the moment the Builder is very Parcel specific... are we sure that we can find a good compromise between all these bundlers? If not then the interface could be just this:

export interface IBuilder {
  readonly handler: string;
  readonly outDir: string;
  build(): void;
}

which allows to do something like this for the constructor of the NodejsFunction

export class NodejsFunction extends lambda.Function {
  constructor(scope: cdk.Construct, id: string, props: NodejsFunctionProps) {
    // ...
    builder.build(); // build it

    super(scope, id, {
      ...props,
      code: lambda.Code.fromAsset(builder.outDir), // where did it build it?
      handler: builder.handler, // what did it build?
    });
  }
}

and use static methods (NodejsFunction.fromParcel(...)). The only thing that would be common is the automatic entry finding and some defaults?

[eladb]

I like @jogold's direction!

[moltar]

This would be perfect for adding webpack support. I am really not liking the Parcel / Docker solution.

[eladb]

@jogold can we close this?

[mrgrain]

Parcel is still built into the NodeJSLambda and despite some abstraction, the constructor still does at lot of generic work that in my opinion could be abstracted.

It's also not yet supporting the pattern NodejsFunction.fromParcel(...) pattern that @jogold suggest above.

Last but not least, there seems to be no indication about the stability of the internal interface (anything except NodejsFunction) so I would be hesitant to re-use them for a custom implementation.

tl;dr The theme of the original feature request was to make it easier to implement different bundling mechanics than the default one. I don't think we are any closer to this yet.

[mrgrain]

I think the recent introduction of the BundlingOptions solves this for me. Closing.
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.BundlingOptions.html

[github-actions[bot]]

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