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

Feature request: NPM workspace support #5236

Closed
snebjorn opened this issue Jun 1, 2023 · 4 comments
Closed

Feature request: NPM workspace support #5236

snebjorn opened this issue Jun 1, 2023 · 4 comments
Labels

Comments

@snebjorn
Copy link

snebjorn commented Jun 1, 2023

Describe your idea/feature/enhancement

I'd like better support for NPM/Yarn workspaces.
Currently a package-lock.json or npm-shrinkwrap.json file present in the CodeUri for your Lambda function is expected. When using UseNpmCi it's required and sam build will fail with this cryptic error

Build Failed
Error: NodejsNpmEsbuildBuilder:EsbuildBundle - Esbuild Failed: ✘ [ERROR] Invalid build flag: "--use-npm-ci"

When using an NPM workspace the folder structure might look like this:

.
+-- node_modules
+-- package-lock.json
+-- package.json
`-- lambdas
   +-- lambda-a
   |   `-- handler.ts
   |   `-- package.json
   |   `-- template.yaml
   +-- lambda-b
   |   `-- handler.ts
   |   `-- package.json
   |   `-- template.yaml

Note that package-lock.json and node_modules are only present in the root.

Proposal

sam-cli should take NPM workspaces into consideration. I'm sure some clever things can be done when multiple lambdas are in a NPM workspace as npm i or npm ci only need to be run once for all node_modules to be installed for all package.json files in the workspace. I believe build can be sped up significantly.

Furthermore the requirement for a package-lock.json or npm-shrinkwrap.json file present in the CodeUri is not true for NPM workspaces. Instead it's required at the NPM workspace root.

Actually running npm i or npm ci in a workspace project, fx ~/<root>/lambdas/lambda-a/> npm ci will do the right thing and install packages in ~/<root>/node_modules/ which is very useful!

This might be a bit of a loose proposal but the goal here is to make it work with NPM workspaces. Not only should it work instead of fail, it should utilize the features of NPM workspaces to speed up build time.

Additional Details

@snebjorn snebjorn added stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. type/feature Feature request labels Jun 1, 2023
@snebjorn
Copy link
Author

snebjorn commented Jun 1, 2023

The mentioned build error wasn't caused by a missing package-lock.json file but instead this aws/aws-sam-build-images#90

@jfuss
Copy link
Contributor

jfuss commented Jun 5, 2023

@snebjorn Thanks for the report. Looks like a change in Lambda Builders that broke this. I don't think this has anything to do with NPM workspaces. I would like to focus this issue on the bug. Once patched, if there is still something missing on the Workspace front, I would ask you to create a new issue so we can scope them correctly.

Updating labels.

@jfuss jfuss added area/build sam build command area/esbuild and removed stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Jun 5, 2023
@mildaniel
Copy link
Contributor

Closing as this bug has now been fixed and released in version 1.86.0 of the CLI. Should anything new arise, please open a new issue.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants