diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index 1090b17b72..1a86f502e1 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -222,6 +222,9 @@ def run(self): if is_sam_template: SamApiProvider.check_implicit_api_resource_ids(self.stacks) + # modify the stack resources to support source maps + self._enable_source_maps() + try: builder = ApplicationBuilder( self.get_resources_to_build(), @@ -265,6 +268,7 @@ def run(self): stack, self._stack_name, self.build_dir, modified_template, build_result ) modified_template = nested_stack_manager.generate_auto_dependency_layer_stack() + move_template(stack.location, output_template_path, modified_template) click.secho("\nBuild Succeeded", fg="green") @@ -307,6 +311,108 @@ def run(self): wrapped_from = deep_wrap if deep_wrap else ex.__class__.__name__ raise UserException(str(ex), wrapped_from=wrapped_from) from ex + def _enable_source_maps(self): + """ + Appends ``NODE_OPTIONS: --enable-source-maps``, if Sourcemap is set to true + and sets Sourcemap to true if ``NODE_OPTIONS: --enable-source-maps`` is provided. + """ + using_source_maps = False + invalid_node_option = False + + for stack in self.stacks: + for name, resource in stack.resources.items(): + metadata = resource.get("Metadata", {}) + if metadata.get("BuildMethod", "") != "esbuild": + continue + + node_option_set = self._is_node_option_set(resource) + + # check if Sourcemap is provided and append --enable-source-map if not set + build_properties = metadata.get("BuildProperties", {}) + source_map = build_properties.get("Sourcemap", None) + + if source_map and not node_option_set: + LOG.info( + "\nSourcemap set without --enable-source-maps, adding" + " --enable-source-maps to function %s NODE_OPTIONS", + name, + ) + + resource.setdefault("Properties", {}) + resource["Properties"].setdefault("Environment", {}) + resource["Properties"]["Environment"].setdefault("Variables", {}) + existing_options = resource["Properties"]["Environment"]["Variables"].setdefault("NODE_OPTIONS", "") + + # make sure the NODE_OPTIONS is a string + if not isinstance(existing_options, str): + invalid_node_option = True + else: + resource["Properties"]["Environment"]["Variables"]["NODE_OPTIONS"] = " ".join( + [existing_options, "--enable-source-maps"] + ) + + using_source_maps = True + + # check if --enable-source-map is provided and append Sourcemap: true if it is not set + if source_map is None and node_option_set: + LOG.info( + "\n--enable-source-maps set without Sourcemap, adding Sourcemap to" + " Metadata BuildProperties for %s", + name, + ) + + resource.setdefault("Metadata", {}) + resource["Metadata"].setdefault("BuildProperties", {}) + resource["Metadata"]["BuildProperties"]["Sourcemap"] = True + + using_source_maps = True + + if using_source_maps: + self._warn_using_source_maps() + + if invalid_node_option: + self._warn_invalid_node_options() + + @staticmethod + def _is_node_option_set(resource: Dict) -> bool: + """ + Checks if the template has NODE_OPTIONS --enable-source-maps set + + Parameters + ---------- + resource : Dict + The resource dictionary to lookup if --enable-source-maps is set + + Returns + ------- + bool + True if --enable-source-maps is set, otherwise false + """ + try: + node_options = resource["Properties"]["Environment"]["Variables"]["NODE_OPTIONS"] + + return "--enable-source-maps" in node_options.split() + except (KeyError, AttributeError): + return False + + @staticmethod + def _warn_invalid_node_options(): + click.secho( + "\nNODE_OPTIONS is not a string! As a result, the NODE_OPTIONS environment variable will " + "not be set correctly, please make sure it is a string. " + "Visit https://nodejs.org/api/cli.html#node_optionsoptions for more details.\n", + fg="yellow", + ) + + @staticmethod + def _warn_using_source_maps(): + click.secho( + "\nYou are using source maps, note that this comes with a performance hit!" + " Set Sourcemap to false and remove" + " NODE_OPTIONS: --enable-source-maps to disable source maps.\n", + fg="yellow", + ) + @staticmethod def gen_success_msg(artifacts_dir: str, output_template_path: str, is_default_build_dir: bool) -> str: diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index dde06145d2..3590f25709 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -61,6 +61,11 @@ def __init__( self.runtime = runtime +class DummyStack: + def __init__(self, resource): + self.resources = resource + + class TestBuildContext__enter__(TestCase): @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @@ -636,8 +641,10 @@ def test_must_print_remote_url_warning( @patch("samcli.commands.build.build_context.move_template") @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") + @patch("samcli.commands.build.build_context.BuildContext._enable_source_maps") def test_run_sync_build_context( self, + source_maps_mock, os_mock, get_template_data_mock, move_template_mock, @@ -892,9 +899,11 @@ class TestBuildContext_run(TestCase): @patch("samcli.commands.build.build_context.move_template") @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") + @patch("samcli.commands.build.build_context.BuildContext._enable_source_maps") def test_run_build_context( self, auto_dependency_layer, + source_map_mock, os_mock, get_template_data_mock, move_template_mock, @@ -947,6 +956,8 @@ def test_run_build_context( ] nested_stack_manager_mock.return_value = given_nested_stack_manager + source_map_mock.side_effect = [modified_template_root, modified_template_child] + with BuildContext( resource_identifier="function_identifier", template_file="template_file", @@ -1060,10 +1071,12 @@ def test_run_build_context( @patch("samcli.commands.build.build_context.move_template") @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") + @patch("samcli.commands.build.build_context.BuildContext._enable_source_maps") def test_must_catch_known_exceptions( self, exception, wrapped_exception, + source_map_mock, os_mock, get_template_data_mock, move_template_mock, @@ -1139,8 +1152,10 @@ def test_must_catch_known_exceptions( @patch("samcli.commands.build.build_context.move_template") @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") + @patch("samcli.commands.build.build_context.BuildContext._enable_source_maps") def test_must_catch_function_not_found_exception( self, + source_map_mock, os_mock, get_template_data_mock, move_template_mock, @@ -1262,3 +1277,166 @@ def test_check_exclude_warning(self, resource_id, exclude, should_print, log_moc log_mock.warning.assert_called_once_with(BuildContext._EXCLUDE_WARNING_MESSAGE) else: log_mock.warning.assert_not_called() + + +class TestBuildContext_is_node_option_set(TestCase): + @parameterized.expand( + [ + ( + {"Properties": {"Environment": {"Variables": {"NODE_OPTIONS": "--enable-source-maps"}}}}, + True, + ), + ( + {"Properties": {"Environment": {"Variables": {"NODE_OPTIONS": "nothing"}}}}, + False, + ), + ] + ) + def test_is_node_option_set(self, resource, expected_result): + build_context = BuildContext( + resource_identifier="resource_id", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode="mode", + ) + + self.assertEqual(build_context._is_node_option_set(resource), expected_result) + + def test_enable_source_map_missing(self): + build_context = BuildContext( + resource_identifier="resource_id", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode="mode", + ) + + self.assertFalse(build_context._is_node_option_set({"Properties": {}})) + + +class TestBuildContext_enable_source_maps(TestCase): + @parameterized.expand( + [ + ({"test": {"Metadata": {"BuildMethod": "esbuild", "BuildProperties": {"Sourcemap": True}}}},), + ( + { + "test": { + "Properties": {"Environment": {"Variables": {"NODE_OPTIONS": "--something"}}}, + "Metadata": {"BuildMethod": "esbuild", "BuildProperties": {"Sourcemap": True}}, + } + }, + ), + ] + ) + @patch("samcli.commands.build.build_context.BuildContext._is_node_option_set") + def test_enable_source_maps_only_source_map(self, resource, is_enable_map_mock): + build_context = BuildContext( + resource_identifier="resource_id", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode="mode", + ) + + stack = DummyStack(resource) + build_context._stacks = [stack] + + is_enable_map_mock.return_value = False + + build_context._enable_source_maps() + + for _, resource in stack.resources.items(): + self.assertIn("--enable-source-maps", resource["Properties"]["Environment"]["Variables"]["NODE_OPTIONS"]) + + @parameterized.expand( + [ + ({"test": {"Metadata": {"BuildMethod": "esbuild"}}}, True, True), + ( + { + "test": { + "Properties": {"Environment": {"Variables": {"NODE_OPTIONS": "--enable-source-maps"}}}, + "Metadata": {"BuildMethod": "esbuild"}, + } + }, + True, + True, + ), + ( + { + "test": { + "Metadata": {"BuildMethod": "esbuild", "BuildProperties": {"Sourcemap": False}}, + } + }, + True, + False, + ), + ] + ) + @patch("samcli.commands.build.build_context.BuildContext._is_node_option_set") + @patch("samcli.commands.build.build_context.get_template_data") + def test_enable_source_maps_only_node_options( + self, resource, node_option_set, expected_value, get_template_mock, is_enable_map_mock + ): + build_context = BuildContext( + resource_identifier="resource_id", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode="mode", + ) + + stack = DummyStack(resource) + build_context._stacks = [stack] + + is_enable_map_mock.return_value = node_option_set + + build_context._enable_source_maps() + + for _, resource in stack.resources.items(): + self.assertEqual(resource["Metadata"]["BuildProperties"]["Sourcemap"], expected_value) + + @patch("samcli.commands.build.build_context.BuildContext._is_node_option_set") + @patch("samcli.commands.build.build_context.BuildContext._warn_using_source_maps") + @patch("samcli.commands.build.build_context.BuildContext._warn_invalid_node_options") + def test_warnings_printed(self, warn_node_option_mock, warn_source_map_mock, is_enable_source_map_mock): + build_context = BuildContext( + resource_identifier="resource_id", + template_file="template_file", + base_dir="base_dir", + build_dir="build_dir", + cache_dir="cache_dir", + cached=False, + parallel=False, + mode="mode", + ) + + stack = DummyStack( + { + "test": { + "Properties": { + "Environment": {"Variables": {"NODE_OPTIONS": ["--something"]}}, + }, + "Metadata": {"BuildMethod": "esbuild", "BuildProperties": {"Sourcemap": True}}, + } + } + ) + build_context._stacks = [stack] + + is_enable_source_map_mock.return_value = False + build_context._enable_source_maps() + + warn_node_option_mock.assert_called() + warn_source_map_mock.assert_called()