feat(lambda-nodejs): add bun support #31770

blimmer · 2024-10-15T22:56:02Z

Issue #31753

Reason for this change

bun is rapidly gaining popularity because it's extremely performant compared to node. bun has its own lockfile, bun.lockb, which is not currently respected by AWS CDK when bundling NodejsFunctions.

Description of changes

This code is very well-structured, so it was simple to add bun support alongside yarn, pnpm and npm.

Description of how you validated changes

I linked this code up to a simple, sample CDK app with a bun lockfile. I wasn't able to bun cdk synth before my changes. Then, after linking up my local workspace, I was able to bundle with my bun lockfile.

I also added unit tests similar to those that exist for the other packages managers.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

blimmer · 2024-10-15T22:59:14Z

packages/aws-cdk-lib/aws-lambda-nodejs/lib/Dockerfile

+# Ensure all users can write to bun cache
+RUN mkdir /tmp/bun-cache && \
+    chmod -R 777 /tmp/bun-cache && \
+    echo -e "[install.cache]\ndir = \"/tmp/bun-cache\"" >> /home/user/.bunfig.toml


There's no bun config set --global command for this cache location. The best place to set this is in the global user homedir according to the docs.

Therefore, this cache statement needs to happen after useradd is called, not above where the other caches are configured. If it's called before, even if I create /home/user, useradd overwrites it.

blimmer · 2024-10-15T23:00:56Z

Exemption Request - I'm following the patterns of existing PRs to add package managers, so I don't feel I need to add an integration test.

blimmer · 2024-10-26T23:21:36Z

@khushail - could I get a review on this? thanks in advance

khushail · 2024-10-28T17:43:35Z

@blimmer , thanks for your contribution. I see this PR has label pr/needs-community-review so it would be great to reach out to CDK Community to get it approved first by the trusted community reviewers and afterwards, the core team should be able to review it. hope that would be helpful!

suds-sky · 2024-11-18T11:18:13Z

Hi @blimmer Great work on picking this up. We are also in need of this. It hasn't been moved for 3 weeks. Do you need any helping hand to proceed with this or it is waiting to find a community member to take a look

blimmer · 2024-11-18T13:46:29Z

Hey @suds-sky - this is awaiting community review. From my perspective it's good to go!

suds-sky · 2024-11-18T14:29:47Z

Hey @suds-sky - this is awaiting community review. From my perspective it's good to go!

Looks like the checks are failing, may be the reason community member is waiting for. There is CDK community Slack, may be we can reach there to get some help 🤔

blimmer · 2024-11-18T14:40:35Z

The failing checks are because I didn't add an integration test. None of the other supported package managers add an integration test, so I felt I didn't need to add one for bun. I've requested an exemption. This is ready for review.

blimmer · 2024-11-18T14:45:43Z

Exemption Request - I'm following the patterns of existing PRs to add package managers, so I don't feel I need to add an integration test.

codecov · 2024-11-18T14:51:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.67%. Comparing base (0ce71fc) to head (f43ae10).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #31770      +/-   ##
==========================================
+ Coverage   78.66%   78.67%   +0.01%     
==========================================
  Files         107      107              
  Lines        7237     7237              
  Branches     1329     1329              
==========================================
+ Hits         5693     5694       +1     
+ Misses       1358     1357       -1     
  Partials      186      186

Flag	Coverage Δ
suite.unit	`78.67% <0.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`78.67% <ø> (+0.01%)`	⬆️

badmintoncryer · 2024-11-24T02:15:43Z

@blimmer Thank you for your great work! I really want to use this feature.
I think it is better to create integ test and snapshots files because this is a new feature PR.

If you have any problem for that, please feel free to contact me.

blimmer · 2024-11-25T16:20:06Z

Thanks for the nudge. I missed that PNPM actually did have an integration test. I added one for bun (and fixed errors found in the integration test 😅). This is ready for review.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

blimmer · 2024-11-25T16:55:26Z

The CodeQL failure is in the generated integration test code, so I think it's a false positive.

badmintoncryer · 2024-11-28T10:38:04Z

@blimmer Thank you for your update. I'll check it later. Please wait for a while.

badmintoncryer

I have only a question.

badmintoncryer · 2024-11-29T12:29:48Z

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-nodejs/test/integ.dependencies-bun.ts

+    // Will be installed, not bundled
+    // (axios is a package with sub-dependencies,
+    // will be used to ensure bun bundling works as expected)
+    nodeModules: ['axios'],


Please let me know: If axios is not listed in bun.lockb, will specifying it in nodeModules still ensure it is installed and works correctly?

If that is the case, I think it would be better to run an integration test to ensure that a Lambda function using modules listed only in bun.lockb works correctly.

This is what's also done in the PNPM integration test:

aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-lambda-nodejs/test/integ.dependencies-pnpm.ts

Lines 16 to 19 in 81cde0e

// Will be installed, not bundled

// (axios is a package with sub-dependencies,

// will be used to ensure pnpm bundling works as expected)

nodeModules: ['axios'],

Axios is listed in bun.lockb, but specifying it in nodeModules installs it using bun inside the docker container (docs). I verified that the sub-dependencies (locked by bun.lockb) were installed properly.

badmintoncryer

Thanks! I'm sorry for my late response.

GavinZZ

Love it! Thanks for adding support for bun.

mergify · 2024-12-04T20:56:37Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2024-12-04T21:01:22Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2024-12-05T19:41:01Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

### Issue aws#31753 Closes aws#31753. ### Reason for this change `bun` is rapidly gaining popularity because it's extremely performant compared to `node`. `bun` has its own lockfile, `bun.lockb`, which is not currently respected by AWS CDK when bundling `NodejsFunction`s. ### Description of changes This code is very well-structured, so it was simple to add bun support alongside `yarn`, `pnpm` and `npm`. ### Description of how you validated changes I linked this code up to a simple, sample CDK app with a `bun` lockfile. I wasn't able to `bun cdk synth` before my changes. Then, after linking up my local workspace, I was able to bundle with my `bun` lockfile. I also added unit tests similar to those that exist for the other packages managers. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

mergify · 2024-12-05T19:44:06Z

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #31770 has been dequeued, merge conditions unmatch:

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

aws-cdk-automation · 2024-12-05T19:46:27Z

The pull request linter fails with the following errors:

❌ Features must contain a change to a README file.
❌ Features must contain a change to a test file.
❌ Features must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

✅ A exemption request has been requested. Please wait for a maintainer's review.

blimmer · 2024-12-05T19:46:47Z

@GavinZZ - any ideas what happened here? It's show 0 files changed... Seems like mergify messed something up.

blimmer · 2024-12-05T19:54:06Z

Oh, bizarre ... it merged but somehow lost being linked up with this PR? aed8ad1

aws-cdk-automation · 2024-12-05T20:14:02Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: f43ae10
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

GavinZZ · 2024-12-05T22:34:39Z

Yes.. That's super weird and my first time seeing any issue like this. But it does seem that the original PR was merged. I'll close this PR since the actual content is merged. Feel free to tag me if you have any additional questions.

github-actions · 2024-12-05T22:34:58Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Oct 15, 2024

aws-cdk-automation requested a review from a team October 15, 2024 22:56

github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Oct 15, 2024

blimmer force-pushed the add-bun-nodejs-function-support branch from a9e0a4e to 8308ec9 Compare October 15, 2024 22:57

aws-cdk-automation previously requested changes Oct 15, 2024

View reviewed changes

blimmer commented Oct 15, 2024

View reviewed changes

blimmer marked this pull request as ready for review October 15, 2024 23:00

aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Oct 15, 2024

blimmer mentioned this pull request Oct 15, 2024

(aws-lambda-nodejs): Support bun lockfiles for NodejsFunction bundling #31753

Closed

2 tasks

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 15, 2024

github-actions bot mentioned this pull request Nov 1, 2024

Monthly PR metrics report #31972

Closed

blimmer added 2 commits November 24, 2024 15:46

feat(lambda-nodejs): add bun support

cfab04c

fix: copy files and add integration test

4f3a72c

blimmer force-pushed the add-bun-nodejs-function-support branch from 22bcd2a to 4f3a72c Compare November 25, 2024 16:19

blimmer and others added 2 commits November 25, 2024 09:22

Merge branch 'main' into add-bun-nodejs-function-support

f248539

fix: tests

f9e685b

badmintoncryer reviewed Nov 29, 2024

View reviewed changes

Merge branch 'main' into add-bun-nodejs-function-support

e44ae4f

badmintoncryer approved these changes Dec 2, 2024

View reviewed changes

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Dec 2, 2024

GavinZZ approved these changes Dec 4, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 4, 2024

Merge branch 'main' into add-bun-nodejs-function-support

79ac170

Merge branch 'main' into add-bun-nodejs-function-support

f43ae10

aws-cdk-automation requested changes Dec 5, 2024

View reviewed changes

GavinZZ closed this Dec 5, 2024

github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2024

blimmer deleted the add-bun-nodejs-function-support branch December 5, 2024 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lambda-nodejs): add bun support #31770

feat(lambda-nodejs): add bun support #31770

blimmer commented Oct 15, 2024

aws-cdk-automation left a comment

blimmer Oct 15, 2024

blimmer commented Oct 15, 2024

blimmer commented Oct 26, 2024

khushail commented Oct 28, 2024

suds-sky commented Nov 18, 2024

blimmer commented Nov 18, 2024

suds-sky commented Nov 18, 2024

blimmer commented Nov 18, 2024

blimmer commented Nov 18, 2024

codecov bot commented Nov 18, 2024 •

edited

Loading

badmintoncryer commented Nov 24, 2024

blimmer commented Nov 25, 2024

blimmer commented Nov 25, 2024

badmintoncryer commented Nov 28, 2024

badmintoncryer left a comment

badmintoncryer Nov 29, 2024

blimmer Nov 30, 2024

badmintoncryer left a comment

GavinZZ left a comment

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 5, 2024

aws-cdk-automation left a comment

mergify bot commented Dec 5, 2024

aws-cdk-automation commented Dec 5, 2024

blimmer commented Dec 5, 2024

blimmer commented Dec 5, 2024

aws-cdk-automation commented Dec 5, 2024

GavinZZ commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

	// Will be installed, not bundled
	// (axios is a package with sub-dependencies,
	// will be used to ensure pnpm bundling works as expected)
	nodeModules: ['axios'],

feat(lambda-nodejs): add bun support #31770

feat(lambda-nodejs): add bun support #31770

Conversation

blimmer commented Oct 15, 2024

Issue #31753

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

blimmer Oct 15, 2024

Choose a reason for hiding this comment

blimmer commented Oct 15, 2024

blimmer commented Oct 26, 2024

khushail commented Oct 28, 2024

suds-sky commented Nov 18, 2024

blimmer commented Nov 18, 2024

suds-sky commented Nov 18, 2024

blimmer commented Nov 18, 2024

blimmer commented Nov 18, 2024

codecov bot commented Nov 18, 2024 • edited Loading

Codecov Report

badmintoncryer commented Nov 24, 2024

blimmer commented Nov 25, 2024

blimmer commented Nov 25, 2024

badmintoncryer commented Nov 28, 2024

badmintoncryer left a comment

Choose a reason for hiding this comment

badmintoncryer Nov 29, 2024

Choose a reason for hiding this comment

blimmer Nov 30, 2024

Choose a reason for hiding this comment

badmintoncryer left a comment

Choose a reason for hiding this comment

GavinZZ left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 5, 2024

aws-cdk-automation left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 5, 2024

aws-cdk-automation commented Dec 5, 2024

blimmer commented Dec 5, 2024

blimmer commented Dec 5, 2024

aws-cdk-automation commented Dec 5, 2024

AWS CodeBuild CI Report

GavinZZ commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

codecov bot commented Nov 18, 2024 •

edited

Loading