-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
chore(lambda-nodejs): local bundling #9632
Conversation
``` | ||
|
||
To force bundling in a Docker container, set the `forceDockerBundling` prop to `true`. This is useful | ||
when installing node modules with native dependencies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence "This is useful when installing node modules with native dependencies" is a bit cryptic. Elaborate a bit or provide an example.
|
||
private static _runsLocally?: boolean; | ||
|
||
constructor(private readonly props: LocalBundlerProps) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to pass all the options here? I recall tryBundle
's second argument is options
which should include all the needed context.
To verify we designed the protocol correctly, let's try to implement tryBundle
through a simple "lambda-interface" instead of as a class:
local: {
tryBundle: (outdir: string, options: ...) => {
// foo bar
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will give it a shot but it seemed easier/cleaner to implement it with the bundler's option instead of parsing the Docker command. Might have implications to offer Windows support for local bundling also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. Yeah I wouldn't parse the docker command...
You are right.
Let's leave as is. I am good
@jogold lmk when this is ready |
It is now |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Install Parcel as a dev dependency to switch to local bundling. aws/aws-cdk#9632
Execute local bundling from the directory containing the entry file. Without this change, in a monorepo with multiple `package.json` files Parcel doesn't look for the right one. Also fix a regression introduced in aws#9632 for the working directory in the container.
…9870) Execute local bundling from the directory containing the entry file. Without this change, in a monorepo with multiple `package.json` files or when consuming a module exposing a construct, Parcel doesn't look for the right `package.json`. Also fix a regression introduced in #9632 for the working directory in the container. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Bundle locally if Parcel v2 is installed.
Closes #9120
Closes #9639
Closes #9153
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license