feat: Install esbuild deps with production flag #379
Conversation
| else: | ||
| # Invalid workflow, can't have no dependency dir and no installation | ||
| raise EsbuildExecutionError(message="Lambda Builders encountered and invalid workflow") | ||
| raise EsbuildExecutionError(message="Lambda Builders encountered an invalid workflow") |
There was a problem hiding this comment.
Should this error be more specific? The comment has more context which could be useful to help customers understand why the build failed. As a customer "invalid workflow" doesn't really tell me much.
| scratch_dir, | ||
| subprocess_npm, | ||
| osutils, | ||
| self.options, |
There was a problem hiding this comment.
I assume is_production defaults to True?
Is there any reason a customer may want False? If so, how would a customer do that?
There was a problem hiding this comment.
Yes, it defaults to true. We don't support any way for customers to set it to False, and we actually don't use it as False anywhere except here so I'm just going to remove that parameter. Setting production=False means that development dependencies will also be installed which adds a ton of bloat and install time that customers likely won't want packaged with their Lambda. Before, we expected customers to include esbuild as a dev dependency. Now, we expect it to be installed on their PATH or as a normal dependency. This is the behaviour we currently have with our Node.js workflow and are making esbuild consistent.
| return NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm) | ||
|
|
||
| return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm, is_production=is_production) | ||
| return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm, is_production=True) |
There was a problem hiding this comment.
We can just remove this parameter.
3 participants
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.