feat: Changed Sourcemap default value to false (#375)

* Changed Sourcemap default to false

* Changed source map DESIGN line to reflect default false change

* Fixed integ tests for esbuild

* Undo change to test exclude sourcemap

Author: lucashuy

aws_lambda_builders/workflows/nodejs_npm_esbuild/DESIGN.md

@@ -117,24 +117,23 @@ testing flow before invoking `sam build`. For additional typescript caveats with
 
 #### Configuring the bundler
 
-The Lambda builder invokes `esbuild` with sensible defaults that will work for the majority of cases. Importantly, the following three parameters are set by default
+The Lambda builder invokes `esbuild` with sensible defaults that will work for the majority of cases. Importantly, the following parameters are set by default
 
 * `--minify`, as it [produces a smaller runtime package](https://esbuild.github.io/api/#minify)
-* `--sourcemap`, as it generates a [source map that allows for correct stack trace reporting](https://esbuild.github.io/api/#sourcemap) in case of errors (see the [Error reporting](#error-reporting) section above)
 * `--target es2020`, as it allows for javascript features present in Node 14
 
 Users might want to tweak some of these runtime arguments for a specific project, for example not including the source map to further reduce the package size, or restricting javascript features to an older version. The Lambda builder allows this with optional sub-properties of the `aws_sam` configuration property.
 
 * `target`: string, corresponding to a supported [esbuild target](https://esbuild.github.io/api/#target) property
 * `minify`: boolean, defaulting to `true`
-* `sourcemap`: boolean, defaulting to `true`
+* `sourcemap`: boolean, defaulting to `false`
 
-Here is an example that deactivates minification and source maps, and supports JavaScript features compatible with Node.js version 10.
+Here is an example that deactivates minification, enables source maps, and supports JavaScript features compatible with Node.js version 10.
 
 ```json
 {
     "entry_points": ["included.ts"],
     "target": "node10",
     "minify": false,
-    "sourcemap": false
+    "sourcemap": true
 }

aws_lambda_builders/workflows/nodejs_npm_esbuild/esbuild.py

@@ -202,9 +202,6 @@ def build_default_values(self) -> "EsbuildCommandBuilder":
         if "minify" not in self._bundler_config:
             args.append("--minify")
 
-        if "sourcemap" not in self._bundler_config:
-            args.append("--sourcemap")
-
         LOG.debug("Using the following default args: %s", str(args))
 
         self._command.extend(args)

tests/integration/workflows/nodejs_npm_esbuild/test_nodejs_npm_with_esbuild.py

@@ -61,7 +61,7 @@ def test_builds_javascript_project_with_dependencies(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js", "included.js.map"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -81,7 +81,7 @@ def test_builds_javascript_project_with_multiple_entrypoints(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js", "included.js.map", "included2.js", "included2.js.map"}
+        expected_files = {"included.js", "included2.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -101,7 +101,7 @@ def test_builds_typescript_projects(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js", "included.js.map"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -129,7 +129,7 @@ def test_builds_with_external_esbuild(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js", "included.js.map"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -165,7 +165,7 @@ def test_bundle_with_implicit_file_types(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js.map", "implicit.js.map", "implicit.js", "included.js"}
+        expected_files = {"implicit.js", "included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -191,7 +191,7 @@ def test_bundles_project_without_dependencies(self, runtime):
             executable_search_paths=[binpath],
         )
 
-        expected_files = {"included.js.map", "included.js"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -219,7 +219,7 @@ def test_builds_project_with_remote_dependencies_without_download_dependencies_w
             executable_search_paths=[binpath],
         )
 
-        expected_files = {"included.js.map", "included.js"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -240,7 +240,7 @@ def test_builds_project_with_remote_dependencies_with_download_dependencies_and_
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js.map", "included.js"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -290,7 +290,7 @@ def test_builds_project_without_combine_dependencies(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js.map", "included.js"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -318,7 +318,7 @@ def test_builds_javascript_project_with_external(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js", "included.js.map"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
         with open(str(os.path.join(self.artifacts_dir, "included.js"))) as f:
@@ -343,7 +343,7 @@ def test_builds_javascript_project_with_loader(self, runtime):
             experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
         )
 
-        expected_files = {"included.js", "included.js.map"}
+        expected_files = {"included.js"}
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
@@ -366,3 +366,23 @@ def test_builds_javascript_project_with_loader(self, runtime):
                 "\turania: astronomy and astrology"
             ),
         )
+
+    @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",)])
+    def test_includes_sourcemap_if_requested(self, runtime):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps-esbuild")
+
+        options = {"entry_points": ["included.js"], "sourcemap": True}
+
+        self.builder.build(
+            source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            os.path.join(source_dir, "package.json"),
+            runtime=runtime,
+            options=options,
+            experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
+        )
+
+        expected_files = {"included.js", "included.js.map"}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEqual(expected_files, output_files)

tests/unit/workflows/nodejs_npm_esbuild/test_actions.py

@@ -60,7 +60,12 @@ def test_raises_error_if_entrypoints_empty_list(self):
 
     def test_packages_javascript_with_minification_and_sourcemap(self):
         action = EsbuildBundleAction(
-            "source", "artifacts", {"entry_points": ["x.js"]}, self.osutils, self.subprocess_esbuild, "package.json"
+            "source",
+            "artifacts",
+            {"entry_points": ["x.js"], "sourcemap": True},
+            self.osutils,
+            self.subprocess_esbuild,
+            "package.json",
         )
         action.execute()
 
@@ -97,7 +102,6 @@ def test_packages_with_externals(self):
                 "--target=es2020",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
                 "--external:fetch",
                 "--external:aws-sdk",
             ],
@@ -123,7 +127,6 @@ def test_packages_with_custom_loaders(self):
                 "--target=es2020",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
                 "--loader:.proto=text",
                 "--loader:.json=js",
             ],
@@ -206,7 +209,6 @@ def test_does_not_minify_if_requested(self):
                 "--outdir=artifacts",
                 "--target=es2020",
                 "--format=cjs",
-                "--sourcemap",
             ],
             cwd="source",
         )
@@ -229,7 +231,6 @@ def test_uses_specified_target(self):
                 "--outdir=artifacts",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
                 "--target=node14",
             ],
             cwd="source",
@@ -254,7 +255,6 @@ def test_includes_multiple_entry_points_if_requested(self):
                 "--outdir=artifacts",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
                 "--target=node14",
             ],
             cwd="source",
@@ -287,7 +287,6 @@ def test_includes_building_with_external_dependencies(self, osutils_mock):
                 "--outdir=artifacts",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
                 "--target=node14",
             ],
             cwd="source",

tests/unit/workflows/nodejs_npm_esbuild/test_esbuild.py

@@ -152,7 +152,6 @@ def test_builds_default_values(self, osutils_mock):
                 "--target=es2020",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
             ],
         )
 
@@ -180,7 +179,6 @@ def test_combined_builder_exclude_all_dependencies(self, osutils_mock):
                 "--target=es2020",
                 "--format=cjs",
                 "--minify",
-                "--sourcemap",
                 "--loader:.proto=text",
                 "--loader:.json=js",
                 "--external:@faker-js/faker",
@@ -237,7 +235,6 @@ def test_combined_builder_with_dependencies(self, osutils_mock):
                 "--outdir=artifacts",
                 "--target=es2020",
                 "--minify",
-                "--sourcemap",
                 "--format=esm",
                 "--loader:.proto=text",
                 "--loader:.json=js",