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: bundling with pnpm and Runtime.NODEJS_20_X fails because of pnpm bug #28318

Open
keisukekomeda opened this issue Dec 10, 2023 · 4 comments
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. effort/medium Medium work item – several days of effort p3

Comments

@keisukekomeda
Copy link

Describe the bug

In Node.js 20, pnpm has a bug around node-fetch.
pnpm/pnpm#6424

This bug leads to the failure in bundling of app using native nodeModuels (like sharp).

Expected Behavior

Successfully synth cdk app.

Current Behavior

Failed to synth cdk app, and show error.
pnpm [ERR_INVALID_THIS]: Value of "this" must be of type URLSearchParams

Reproduction Steps

Here is the minumum repro.
https://github.com/keisukekomeda/aws-cdk-aws_lambda_nodejs-pnpm-repro

# enbale pnpm
corepack enable
# install node modules
pnpm install
# synth cdk app (should fails)
pnpm cdk synth

Possible Solution

I found that using pnpm 8.12.0 resolves the error.

As a workaround, I use a custom dockerImage for bundling.
I think I'd like to update the pnpm version in default docker image.

    new cdk.aws_lambda_nodejs.NodejsFunction(this, "PnpmRepro", {
      runtime: cdk.aws_lambda.Runtime.NODEJS_20_X,
      entry: path.resolve(__dirname, "../src/index.ts"),
      handler: "main",
      bundling: {
        forceDockerBundling: true,
        nodeModules: ["sharp"],
        dockerImage: cdk.DockerImage.fromBuild(path.join(__dirname, "."), {
          buildArgs: {
            IMAGE: cdk.aws_lambda.Runtime.NODEJS_20_X.bundlingImage.image,
            ESBUILD_VERSION: "0",
          },
        }),
      }
    })
# The correct AWS SAM build image based on the runtime of the function will be
# passed as build arg. The default allows to do `docker build .` when testing.
ARG IMAGE=public.ecr.aws/sam/build-nodejs18.x
FROM $IMAGE

# Install yarn
RUN npm install --global yarn@1.22.5

# Install pnpm
RUN npm install --global pnpm@8.12.0 # <--- changed!!!!!

# Install typescript
RUN npm install --global typescript

# Install esbuild
# (unsafe-perm because esbuild has a postinstall script)
ARG ESBUILD_VERSION=0
RUN npm install --global --unsafe-perm=true esbuild@$ESBUILD_VERSION

# Ensure all users can write to npm cache
RUN mkdir /tmp/npm-cache && \
    chmod -R 777 /tmp/npm-cache && \
    npm config --global set cache /tmp/npm-cache

# Ensure all users can write to yarn cache
RUN mkdir /tmp/yarn-cache && \
    chmod -R 777 /tmp/yarn-cache && \
    yarn config set cache-folder /tmp/yarn-cache

# Ensure all users can write to pnpm cache
RUN mkdir /tmp/pnpm-cache && \
    chmod -R 777 /tmp/pnpm-cache && \
    pnpm config --global set store-dir /tmp/pnpm-cache

# Disable npm update notifications
RUN npm config --global set update-notifier false

# create non root user and change allow execute command for non root user
RUN /sbin/useradd -u 1000 user && chmod 711 /

CMD [ "esbuild" ]

Additional Information/Context

No response

CDK CLI Version

2.114.1 (build 02bbb1d)

Framework Version

No response

Node.js Version

v20.10.0

OS

MacOS 14.1.2(23B92)

Language

TypeScript

Language Version

TypeScript (5.2.2)

Other information

No response

@keisukekomeda keisukekomeda added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 10, 2023
@keisukekomeda
Copy link
Author

I think pnpm version can be updated to v8 because Node.js 14 has been deprecated.

#24837

@pahud
Copy link
Contributor

pahud commented Dec 11, 2023

Thank you for your report. Please note CDK is not fully tested with pnpm.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 11, 2023
@keisukekomeda
Copy link
Author

@pahud
Thank you for your reply.

I am willing to create PR for this, but I would like to wait for more feedback from the community.

I think we can upgrade pnpm to v8.
reasons...

  • pnpm v7 will be maintained at least until Mar 2024 (not so far).
  • There are some Major Changes between pnpm v7 and v8, but all of them are acceptable as bundling environment.

@pahud
Copy link
Contributor

pahud commented Dec 13, 2023

Yeah, I agree this deserves more community discussion and feedback.

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. effort/medium Medium work item – several days of effort p3
Projects
None yet
Development

No branches or pull requests

2 participants