aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.5k stars 3.84k forks source link

aws-lambda-nodejs: preCompilation flag: npm run build creates different *.js then npm run cdk diff/deploy #21168

Open guentherwieser opened 2 years ago

guentherwieser commented 2 years ago

Describe the bug

I'm using the preCompilation flag on the NodejsFunction (version 2.32.0) and the emitDecoratorMetada: true flag as I need reflect-metadata on the transpiled code. Running npm run build creates the necessary kind of metadata on the resulting js files, but when calling npm run cdk deploy or npm run cdk diff, the js file in the resulting assets does not contain the kind of metadata. According to https://docs.aws.amazon.com/cdk/v2/guide/work-with-cdk-typescript.html#typescript-local I would have assumed that the generation of the js files is basically the same in both npm usages (run build vs. run cdk xxx).

Expected Behavior

Generate the same metadata info when CDK takes care of asset creation as when using npm run build.

Current Behavior

Generated index.js in the asset folder of the CDK outupt doesn't contain the expected metadata.

Reproduction Steps

Use NodejsFunction in 2.32.0 and Typescript files which use decorators and reflect-metadata, and set bundling: { preCompilation: true, },

Additionally, tsconfig has to have "emitDecoratorMetadata": true

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.32.0

Framework Version

2.32.0

Node.js Version

v18.5.0

OS

Mac OS 13 beta 3

Language

Typescript

Language Version

3.9.7

Other information

I'm using 2.32.0 as there were problems around "preCompilation" (see #18002 which this one here might be related).

hassanazharkhan commented 2 years ago

@NoComment73 Thanks for opening the issue, Just to confirm, Does your tsconfig includes the following flags that are required by tsc to include metadata in resulting *.js files?

"emitDecoratorMetadata": true,
"experimentalDecorators": true,

preCompilation actually uses your tsconfig and runs a tsc based on the configurations you have, if the resulting files do not include metadata I suspect the issue is with your configuration. Happy to dig down into this if you could provide a sample repo that addresses this bug.

guentherwieser commented 2 years ago

Yes, my tsconfig contains these settings. When compiling the code with tsc everything gets generated as expected, but not when using CDK/esbuild.

I will create a repo and post it here.

Here's the tsconfig.js, just in case something else in these settings might impact it, and is obvious to your trained eye:

{
    "compilerOptions": {
        "target": "ES2018",
        "module": "commonjs",
        "lib": [
            "es2018",
            "dom"
        ],
        "declaration": true,
        "strict": true,
        "noImplicitAny": true,
        "strictNullChecks": true,
        "noImplicitThis": true,
        "alwaysStrict": true,
        "noUnusedLocals": false,
        "noUnusedParameters": false,
        "noImplicitReturns": false,
        "noFallthroughCasesInSwitch": false,
        "inlineSourceMap": true,
        "inlineSources": true,
        "experimentalDecorators": true,
        "strictPropertyInitialization": true,
        "typeRoots": [
            "./node_modules/@types"
        ],
        "emitDecoratorMetadata": true,
        "allowSyntheticDefaultImports": true
    },
    "exclude": [
        "cdk.out"
    ]
}
guentherwieser commented 2 years ago

@hassanazharkhan here's the repo: https://github.com/NoComment73/aws-cdk-preCompilation-bug-21168

Sorry for the "mess" in it but it's basically an excerpt of our test repo to solve this issue.

You might need to run cdk like this: STACK_BUILD_TARGET_ACCT=gw npm run cdk diff

corymhall commented 2 years ago

@NoComment73 can you provide details on what type of metadata you are expecting vs what you are getting? Looking at the asset that is generated in your linked project I do see it using the reflect-metadata module.

guentherwieser commented 2 years ago

In the files compiled with TSC you should see something similar like this:

__decorate([
    common_1.Attribute(),
    __metadata("design:type", String)
], DbConnectorDefinition.prototype, "name", void 0);

When you search for "design:type" in the generated index.js, you will only find this string as part of querying for metadata, or throwing an exception (6 occurences). But it misses all of the entries that should decorate the model attributes.

corymhall commented 2 years ago

I know the documentation mentions this specific use case for using preCompilation with tsc, but I do not actually know if it will work. Will esbuild still compile or does it just use what was compiled from tsc? @jogold do you know?

jogold commented 2 years ago

Will esbuild still compile or does it just use what was compiled from tsc?

esbuild will use what was compiled from tsc

guentherwieser commented 2 years ago

Re-tested with 2.38.1 (hoping for a miracle ;-) ), still same behavior

keikumata commented 1 year ago

I had the same issues using typedi and typegraphql - the dependency injection code was partially missing in the final esbuild bundle, which meant that the injection logic didn't work at all.

Transpiled code from tsc

SomethingResolver = __decorate([
    (0, typedi_1.Service)(),
    (0, type_graphql_1.Resolver)(some_1.Something),
    __param(0, (0, typedi_1.Inject)("SomeClient")),
    __metadata("design:paramtypes", [some_1.SomeClient])
], SomethingResolver);

Bundled code via esbuild

SomethingResolver = __decorate([
      (0, typedi_1.Service)(),
      (0, type_graphql_1.Resolver)(some_1.Something),
      __param(0, (0, typedi_1.Inject)("SomeClient")), // missing the metadata with design:paramtypes
    ], SomethingResolver);

Solution

I was able to solve this by adding the following to the NodeJsFunction params:

this.lambdaFunction = new NodejsFunction(this, "LambdaFunction", {
    ...
    bundling: {
        preCompilation: true,
        esbuildArgs: {
            "--resolve-extensions": ".js",
        },
    }
});

For some reason, when esbuild tries to bundle the files as is after the tsc transpilation, we lose out on some of the decorator logic that correctly existed in the transpiled js files from tsc. This is similar to the issue mentioned above: https://github.com/aws/aws-cdk/issues/21168#issuecomment-1210543011.

While I'm not sure why, empirically it seems like this has something to do with the fact that esbuild prioritizes .ts files in the same directory over .js files (ref: https://github.com/evanw/esbuild/issues/1295#issuecomment-845536042), and when bundling with .ts files, the final output is incorrect.

Once I added --resolve-extensions=.js files to target only the .js files then everything worked perfectly.

This is basically doing the same thing as what's mentioned here: https://github.com/aws/aws-cdk/pull/16543#issuecomment-934263816. Perhaps it's worth adding in the docs that you may need to add "--resolve-extensions": ".js" for everything to work.

guentherwieser commented 1 year ago

Thx @keikumata for sharing, this gives me hope :-) I'm not sure where to add the "resolve-extension" parameter when using CDK and NodeJsFunction - there is no webpack involved (assuming resolve-extensions is a webpack parameter, at least this was the only usage I was able to find on Google). Thx a lot in advance.

guentherwieser commented 1 year ago

@keikumata Sorry for bothering you, I solved this by adding the parameter to the esbuildArgs of the "bundling" option in NodeJsFunction properties like so:

this.queryDbLambda = new NodejsFunction(stack, 'QueryDbLambda', {
                bundling: {
                    preCompilation: true,
                    esbuildArgs: {
                        '--resolve-extensions': '.js'
                    },
                },...

With this, the decorator stuff that adds the metadata appears in the generated index.js - fantastic!

Nevertheless, this should either be documented as a solution or fixed in the code.

keikumata commented 1 year ago

@NoComment73 glad that it worked for you as well!! this took me a while to figure out so I'm happy to have helped :)

I still don't understand fully why this works though - it'd be easier to understand if none of the decorator stuff was showing up in the final bundled file, but some show up and others don't. Would love input from the community so I can understand this better :)

nevace commented 1 year ago

Works for me! Been stuck on this for hours and was in the middle of writing a custom script for esbuild.

patrickcollings commented 1 year ago

@keikumata Thank you for this! Have been stuck for days so going to bump for visibility as this was not documented anywhere. For anyone else trying to use typeorm then this was the solution that worked for me.

dwright20 commented 1 month ago

This is still relevant and should 100% be added as a note in the docs