Skip to content

feat(lambda-nodejs): source map mode#15621

Merged
mergify[bot] merged 8 commits intoaws:masterfrom
1337vak:add-bundle-mode
Jul 20, 2021
Merged

feat(lambda-nodejs): source map mode#15621
mergify[bot] merged 8 commits intoaws:masterfrom
1337vak:add-bundle-mode

Conversation

@1337vak
Copy link
Copy Markdown
Contributor

@1337vak 1337vak commented Jul 17, 2021

Addresses #14857 by adding source map configuration options. Current implementation preferred preserving backwards compatibility with sourceMap boolean flag. See issue discussion for an alternative.


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

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Jul 17, 2021

eladb
eladb previously requested changes Jul 18, 2021
* @see https://esbuild.github.io/api/#sourcemap
*/
export enum SourceMapMode {
/** Default sourceMap mode */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add docstrings from esbuild explaining what each option means

@1337vak 1337vak requested a review from eladb July 18, 2021 18:10
@mergify mergify bot dismissed eladb’s stale review July 18, 2021 18:10

Pull request has been modified.

@eladb eladb changed the title feat(lambda-nodejs): add bundling mode feat(lambda-nodejs): source map mode Jul 19, 2021
eladb
eladb previously requested changes Jul 19, 2021
});
});

test('esbuild bundling throws when sourceMapMode used with falsy sourceMap', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I actually that setting sourceMapMode to a value should imply sourceMap: true (unless explicitly set to false and then I'd expect an error).

The API design tenet is that if you can do the right thing based on the user intent, why yell at them?

haimlit and others added 2 commits July 19, 2021 18:57
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
@mergify mergify bot dismissed eladb’s stale review July 19, 2021 15:58

Pull request has been modified.

@1337vak 1337vak requested a review from eladb July 19, 2021 16:16
Copy link
Copy Markdown
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

👍

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jul 20, 2021

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-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 7cbb702
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit b934976 into aws:master Jul 20, 2021
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jul 20, 2021

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).

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Aug 3, 2021
Addresses aws#14857 by adding source map configuration options. Current implementation preferred preserving backwards compatibility with `sourceMap` boolean flag. See issue discussion for an alternative.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this pull request Aug 26, 2021
Addresses aws#14857 by adding source map configuration options. Current implementation preferred preserving backwards compatibility with `sourceMap` boolean flag. See issue discussion for an alternative.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants