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

aws-lambda-nodejs: package.json type field is not preserved with external node_modules #32777

Open
1 task
ben-eb opened this issue Jan 7, 2025 · 0 comments
Open
1 task
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@ben-eb
Copy link
Contributor

ben-eb commented Jan 7, 2025

Describe the bug

Hello 👋

I am using https://knexjs.org/ to apply database migrations via Lambda. This requires some modules to be loaded externally due to dynamic import which is working well with CommonJS.

However when porting the code to ES Modules (and using OutputFormat.ESM) I am seeing some module loading issues due to the type key not being set in the generated package.json file. I traced this back to this line:

osCommand.writeJson(pathJoin(options.outputDir, 'package.json'), { dependencies }),

This is surprising as it implicitly changes the output format of the bundle back to CommonJS.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

The type property should be added to the package.json file that is generated when using the nodeModules option, but only when in the source package.json file.

Current Behavior

No type property is added to the generated package.json.

Reproduction Steps

import * as cdk from 'aws-cdk-lib';
import { type Construct } from 'constructs';
import { NodejsFunction, OutputFormat } from 'aws-cdk-lib/aws-lambda-nodejs';
import * as Lambda from 'aws-cdk-lib/aws-lambda';

export class InfrastructureStack extends cdk.Stack {
    constructor(scope: Construct, id: string) {
        super(scope, id);

        const banner =
            "const require = (await import('node:module')).createRequire(import.meta.url);const __filename = (await import('node:url')).fileURLToPath(import.meta.url);const __dirname = (await import('node:path')).dirname(__filename);";
        
        new NodejsFunction(this, 'lambda', {
            functionName: 'test-knex-esm',
            entry: 'index.ts',
            runtime: Lambda.Runtime.NODEJS_20_X,
            bundling: {
                banner,
                format: OutputFormat.ESM,
                mainFields: ['module', 'main'],
                nodeModules: ['knex'],
            }
        })
    }
}

index.ts can have any contents to replicate this issue.

Possible Solution

The type property is also added to the generated package.json when using the nodeModules option and only if it is present in the source package.json. However it may be worth considering to be explicit here as per the Node.js recommendation (and setting the type based on the value of OutputType):

Writing ES module syntax in "ambiguous" files incurs a performance cost, and therefore it is encouraged that authors be explicit wherever possible. In particular, package authors should always include the "type" field in their package.json files, even in packages where all sources are CommonJS. Being explicit about the type of the package will future-proof the package in case the default type of Node.js ever changes, and it will also make things easier for build tools and loaders to determine how the files in the package should be interpreted.

https://nodejs.org/api/packages.html#determining-module-system

Additional Information/Context

There is a workaround here which is to write the source code in .mjs or .mts explicitly and not rely on the package.json lookup to determine the default module system that .js uses.

CDK CLI Version

v2.174.0

Framework Version

v2.174.0

Node.js Version

v20.18.0

OS

Mac OS X

Language

TypeScript

Language Version

TypeScript 5.7.2

Other information

No response

@ben-eb ben-eb added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

1 participant