-
Notifications
You must be signed in to change notification settings - Fork 136
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
fix: Ignore flags not relevant to esbuild #510
Conversation
@@ -101,6 +101,10 @@ def run(self, args, cwd=None): | |||
|
|||
NON_CONFIGURABLE_VALUES = {"bundle", "platform", "outdir"} | |||
|
|||
# Ignore the values below. These are options that Lambda Builders accepts for | |||
# Node.js related workflows, but are not relevant to esbuild itself. | |||
ESBUILD_IGNORE_VALUES = {"use_npm_ci", "entry_points"} |
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 explored other ways of ignoring these types of unrelated values, however didn't find anything reliable (e.g. there's no API to use, and parsing the esbuild help text is very complicated). We can go with this solution for now as it fixes the immediate issue, and look for ways of making this more reliable moving forward.
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.
Change seems reasonable to me.
Are we sure these are the only two (use_npm_ci
, entry_points
)?
Yup, should be just these. |
Issue #, if available:
aws/aws-sam-build-images#90
aws/aws-sam-cli#5236
Description of changes:
Since making the change to accept any flags for esbuild, there is an issue with esbuild trying to use flags that are not related to it. This changes forces the esbuild command builder to ignore those unrelated flags.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.