Skip to content

Commit 1d13b9c

Browse files
authored
refactor: npm_translate_lock_generate.bzl cleanup (#2453)
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
1 parent a34f4d9 commit 1d13b9c

File tree

2 files changed

+12
-14
lines changed

2 files changed

+12
-14
lines changed

npm/private/npm_translate_lock_generate.bzl

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
222222
{validation_call}
223223
""".format(
224224
defs_bzl_file = "@{}//:{}".format(rctx.name, rctx.attr.defs_bzl_filename),
225-
link_packages_comma_separated = "\"'\" + \"', '\".join(_IMPORTER_PACKAGES) + \"'\"" if len(package_to_importer) else "\"\"",
225+
link_packages_comma_separated = "\"'\" + \"', '\".join(_IMPORTER_PACKAGES) + \"'\"" if package_to_importer else "\"\"",
226226
root_package = root_package,
227227
pnpm_lock_label = rctx.attr.pnpm_lock,
228228
validation_call = validation_call,
@@ -386,9 +386,9 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
386386
))
387387

388388
# Generate a single _FP_DIRECT_TMPL block with all link packages
389-
if len(fp_link["link_packages"]) > 0:
389+
if fp_link["link_packages"]:
390390
package_visibility, _ = helpers.gather_values_from_matching_names(True, rctx.attr.package_visibility, "*", fp_package)
391-
if len(package_visibility) == 0:
391+
if not package_visibility:
392392
package_visibility = None
393393

394394
link_factories_bzl.append(_FP_DIRECT_TMPL.format(
@@ -405,7 +405,7 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
405405
links_pkg_bzl[link_package] = []
406406
links_pkg_bzl[link_package].append(""" _fp_link_{i}(name)""".format(i = i))
407407

408-
if len(stores_bzl) > 0:
408+
if stores_bzl:
409409
npm_link_all_packages_bzl.append(""" if is_root:""")
410410
npm_link_all_packages_bzl.extend(stores_bzl)
411411

@@ -416,7 +416,7 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
416416
""")
417417

418418
# Invoke and collect link targets based on package
419-
if len(links_pkg_bzl) > 0:
419+
if links_pkg_bzl:
420420
npm_link_all_packages_bzl.append(""" if link:""")
421421
first_link = True
422422
for link_package, bzl in links_pkg_bzl.items():
@@ -426,12 +426,12 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
426426
))
427427
npm_link_all_packages_bzl.extend(bzl)
428428

429-
if link_package in links_targets and (len(links_targets[link_package]["prod"]) > 0 or len(links_targets[link_package]["dev"]) > 0):
429+
if link_package in links_targets and (links_targets[link_package]["prod"] or links_targets[link_package]["dev"]):
430430
npm_link_all_packages_bzl.append(""" link_targets = {targets}""".format(
431431
targets = starlark_codegen_utils.to_list_attr(links_targets[link_package]["prod"] + links_targets[link_package]["dev"], 3, 4, quote_value = False),
432432
))
433433

434-
if link_package in links_scope_targets and len(links_scope_targets[link_package]) > 0:
434+
if links_scope_targets.get(link_package):
435435
npm_link_all_packages_bzl.append(""" scope_targets = {targets}""".format(
436436
targets = starlark_codegen_utils.to_dict_list_attr(links_scope_targets[link_package], 3, 4, quote_list_value = False),
437437
))
@@ -565,7 +565,7 @@ def _gen_npm_import(rctx, _import, link_workspace):
565565
maybe_deps = ("""
566566
deps = %s,""" % starlark_codegen_utils.to_dict_attr(_import.deps, 2)) if _import.deps else ""
567567
maybe_transitive_closure = ("""
568-
transitive_closure = %s,""" % starlark_codegen_utils.to_dict_list_attr(_import.transitive_closure, 2)) if len(_import.transitive_closure) > 0 else ""
568+
transitive_closure = %s,""" % starlark_codegen_utils.to_dict_list_attr(_import.transitive_closure, 2)) if _import.transitive_closure else ""
569569
maybe_patch_tool = ("""
570570
patch_tool = "%s",""" % _import.patch_tool) if _import.patch_tool else ""
571571
maybe_patches = ("""
@@ -702,13 +702,14 @@ def _generate_npm_package_locations(fp_links, npm_imports):
702702
for _import in npm_imports:
703703
if _import.link_packages:
704704
for link_package, link_aliases in _import.link_packages.items():
705-
aliases_to_add = link_aliases if link_aliases else [_import.package]
705+
aliases_to_add = link_aliases or [_import.package]
706706
for alias in aliases_to_add:
707707
if link_package not in location_to_packages:
708708
location_to_packages[link_package] = []
709709
if alias not in location_to_packages[link_package]:
710710
location_to_packages[link_package].append(alias)
711711

712+
# TODO(zbarsky): `sorted_map` could just return the dict's items which is what `to_dict_attr` wants anyway.
712713
location_to_packages = utils.sorted_map(location_to_packages)
713714

714715
return "_NPM_PACKAGE_LOCATIONS = {}".format(

npm/private/utils.bzl

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,8 @@ DEFAULT_REGISTRY_PROTOCOL = "https"
1111
DEFAULT_EXTERNAL_REPOSITORY_ACTION_CACHE = ".aspect/rules/external_repository_action_cache"
1212

1313
def _sorted_map(m):
14-
result = dict()
15-
for key in sorted(m.keys()):
16-
result[key] = m[key]
17-
18-
return result
14+
# TODO(zbarsky): maybe faster as `dict(sorted(m.items()))`?
15+
return {k: m[k] for k in sorted(m.keys())}
1916

2017
def _sanitize_rule_name(string):
2118
# Workspace names may contain only A-Z, a-z, 0-9, '-', '_' and '.'

0 commit comments

Comments
 (0)