Skip to content

Conversation

@chris-armstrong
Copy link
Contributor

@chris-armstrong chris-armstrong commented Mar 13, 2022

This PR adds support to the esbuild BuildProperties to specify a set of externals to use when building a source code bundle. This is useful when the package you require is on a layer and cannot be bundled with esbuild. It converts an array of Externals to a set of esbuild --external:<module> parameters.

For example, if your BuildProperties contains:

  MyFunction:
    Metadata:
      BuildProperties:
        Externals: ['aws-sdk', 'puppeteer']

Then --external:aws-sdk --external:puppeteer will be added to the esbuild command line.

(Please forgive any non-idiomatic Python code - this is the first time writing Python for a number of years so I'm rusty on latest best practice).

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

CoshUS and others added 7 commits September 29, 2021 17:02
chore: Merge develop into master for 1.8.1 Release
chore: Lambda Builder 1.9.0 Release Merge to Master
chore: Merge develop into master
chore: Merge develop into master
chore: Merge develop into master
args.append("--minify")
if sourcemap:
args.append("--sourcemap")
if len(externals) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor comment, but can we make this if externals: to be consistent with PEP8 styling guide?

Copy link
Contributor

@mndeveci mndeveci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chris-armstrong thanks for the PR! Can you address the concern from @mildaniel? Other than that, LGTM.

@chris-armstrong
Copy link
Contributor Author

@chris-armstrong thanks for the PR! Can you address the concern from @mildaniel? Other than that, LGTM.

The code uses ‘if externals:’ now - was there anything else?

Copy link
Contributor

@mndeveci mndeveci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chris-armstrong thanks for the PR! Can you address the concern from @mildaniel? Other than that, LGTM.

The code uses ‘if externals:’ now - was there anything else?

Sorry my bad!

@mndeveci mndeveci changed the base branch from master to develop April 14, 2022 01:37
@mndeveci
Copy link
Contributor

mndeveci commented Apr 14, 2022

@chris-armstrong one last thing. This PR was pointing to master branch. I now changed it to develop, but it is out of date now. Can you re-base your changes from develop so that we can merge into?

If you want, you can close this PR and open a new one, which should branch out from develop.

@mndeveci mndeveci self-requested a review April 14, 2022 01:39
@chris-armstrong
Copy link
Contributor Author

I can, but I’ll be away from my computer for the next few days so I won’t be able to rebase immediately

@chris-armstrong
Copy link
Contributor Author

@chris-armstrong one last thing. This PR was pointing to master branch. I now changed it to develop, but it is out of date now. Can you re-base your changes from develop so that we can merge into?

If you want, you can close this PR and open a new one, which should branch out from develop.

I've opened #353 that has been rebased against develop

@moelasmar
Copy link
Contributor

@chris-armstrong .. should we close this PR?

@chris-armstrong
Copy link
Contributor Author

@chris-armstrong .. should we close this PR?

Yes I'll close this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants