Skip to content

refactor: esbuild refactor for readability #433

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

Merged
merged 14 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 80 additions & 77 deletions aws_lambda_builders/workflows/nodejs_npm_esbuild/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
LOG = logging.getLogger(__name__)

EXTERNAL_KEY = "external"
# minimum esbuild version required to use "--external"
MINIMUM_VERSION_FOR_EXTERNAL = "0.14.13"


class EsbuildBundleAction(BaseAction):
Expand All @@ -27,40 +29,36 @@ class EsbuildBundleAction(BaseAction):

def __init__(
self,
scratch_dir: str,
artifacts_dir: str,
working_directory: str,
output_directory: str,
Comment on lines +32 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed to make more general

bundler_config: Dict[str, Any],
osutils: OSUtils,
subprocess_esbuild: SubprocessEsbuild,
manifest: str,
skip_deps=False,
):
"""
:type scratch_dir: str
:param scratch_dir: an existing (writable) directory for temporary files

:type artifacts_dir: str
:param artifacts_dir: an existing (writable) directory where to store the output.
Parameters
----------
working_directory : str
directory where esbuild is executed
output_directory : str
an existing (writable) directory where to store the output.
Note that the actual result will be in the 'package' subdirectory here.

:type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils
:param osutils: An instance of OS Utilities for file manipulation

:type subprocess_esbuild: aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild
:param subprocess_esbuild: An instance of the Esbuild process wrapper

:type skip_deps: bool
:param skip_deps: if dependencies should be omitted from bundling

:type bundler_config: Dict[str,Any]
:param bundler_config: the bundler configuration

:type manifest: str
:param manifest: path to package.json file contents to read
bundler_config : Dict[str, Any]
the bundle configuration
osutils : OSUtils
An instance of OS Utilities for file manipulation
subprocess_esbuild : SubprocessEsbuild
An instance of the Esbuild process wrapper
manifest : str
path to package.json file contents to read
skip_deps : bool, optional
if dependencies should be omitted from bundling, by default False
"""
super(EsbuildBundleAction, self).__init__()
self._scratch_dir = scratch_dir
self._artifacts_dir = artifacts_dir
self._working_directory = working_directory
self._output_directory = output_directory
self._bundler_config = bundler_config
self._osutils = osutils
self._subprocess_esbuild = subprocess_esbuild
Expand All @@ -71,13 +69,21 @@ def execute(self) -> None:
"""
Runs the action.

:raises lambda_builders.actions.ActionFailedError: when esbuild packaging fails
Raises
------
ActionFailedError
when esbuild packaging fails
"""
esbuild_command = EsbuildCommandBuilder(
self._scratch_dir, self._artifacts_dir, self._bundler_config, self._osutils, self._manifest
self._working_directory, self._output_directory, self._bundler_config, self._osutils, self._manifest
)

if self._should_bundle_deps_externally():
check_minimum_esbuild_version(
minimum_version_required=MINIMUM_VERSION_FOR_EXTERNAL,
working_directory=self._working_directory,
subprocess_esbuild=self._subprocess_esbuild,
)
Comment on lines +83 to +86
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to be a separate action. Since we need to check the version every time that we want to mark things as --external then I think we should do the check when/before marking things as external (here), instead of having a separate action that would always have to be appended before this one. This cleans things up a bit, but it's also safer as we don't have to remember that we need a check action before this bundle action.

esbuild_command.build_with_no_dependencies()
if EXTERNAL_KEY in self._bundler_config:
# Already marking everything as external,
Expand All @@ -89,7 +95,7 @@ def execute(self) -> None:
)

try:
self._subprocess_esbuild.run(args, cwd=self._scratch_dir)
self._subprocess_esbuild.run(args, cwd=self._working_directory)
except EsbuildExecutionError as ex:
raise ActionFailedError(str(ex))

Expand All @@ -103,65 +109,62 @@ def _should_bundle_deps_externally(self) -> bool:
return self._skip_deps or "./node_modules/*" in self._bundler_config.get(EXTERNAL_KEY, [])


class EsbuildCheckVersionAction(BaseAction):
"""
A Lambda Builder Action that verifies that esbuild is a version supported by sam accelerate
def check_minimum_esbuild_version(
minimum_version_required: str, working_directory: str, subprocess_esbuild: SubprocessEsbuild
):
"""
Checks esbuild version against a minimum version required.

NAME = "EsbuildCheckVersion"
DESCRIPTION = "Checking esbuild version"
PURPOSE = Purpose.COMPILE_SOURCE
Parameters
----------
minimum_version_required: str
minimum esbuild version required for check to pass

MIN_VERSION = "0.14.13"
working_directory: str
directory where esbuild is executed

def __init__(self, scratch_dir, subprocess_esbuild):
"""
:type scratch_dir: str
:param scratch_dir: temporary directory where esbuild is executed
subprocess_esbuild: aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild
An instance of the Esbuild process wrapper

:type subprocess_esbuild: aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild.SubprocessEsbuild
:param subprocess_esbuild: An instance of the Esbuild process wrapper
"""
super().__init__()
self.scratch_dir = scratch_dir
self.subprocess_esbuild = subprocess_esbuild
Raises
----------
lambda_builders.actions.ActionFailedError
when esbuild version checking fails
"""
args = ["--version"]

def execute(self):
"""
Runs the action.
try:
version = subprocess_esbuild.run(args, cwd=working_directory)
except EsbuildExecutionError as ex:
raise ActionFailedError(str(ex))

:raises lambda_builders.actions.ActionFailedError: when esbuild version checking fails
"""
args = ["--version"]
LOG.debug("Found esbuild with version: %s", version)

try:
version = self.subprocess_esbuild.run(args, cwd=self.scratch_dir)
except EsbuildExecutionError as ex:
raise ActionFailedError(str(ex))
try:
check_version = _get_version_tuple(minimum_version_required)
esbuild_version = _get_version_tuple(version)

LOG.debug("Found esbuild with version: %s", version)
if esbuild_version < check_version:
raise ActionFailedError(
f"Unsupported esbuild version. To use a dependency layer, the esbuild version must be at "
f"least {minimum_version_required}. Version found: {version}"
)
except (TypeError, ValueError) as ex:
raise ActionFailedError(f"Unable to parse esbuild version: {str(ex)}")

try:
check_version = EsbuildCheckVersionAction._get_version_tuple(self.MIN_VERSION)
esbuild_version = EsbuildCheckVersionAction._get_version_tuple(version)

if esbuild_version < check_version:
raise ActionFailedError(
f"Unsupported esbuild version. To use a dependency layer, the esbuild version must be at "
f"least {self.MIN_VERSION}. Version found: {version}"
)
except (TypeError, ValueError) as ex:
raise ActionFailedError(f"Unable to parse esbuild version: {str(ex)}")

@staticmethod
def _get_version_tuple(version_string):
"""
Get an integer tuple representation of the version for comparison

:type version_string: str
:param version_string: string containing the esbuild version
def _get_version_tuple(version_string: str):
"""
Get an integer tuple representation of the version for comparison

:rtype: tuple
:return: version tuple used for comparison
"""
return tuple(map(int, version_string.split(".")))
Parameters
----------
version_string: str
string containing the esbuild version

Returns
----------
tuple
version tuple used for comparison
"""
return tuple(map(int, version_string.split(".")))
Loading