Skip to content

Commit

Permalink
chore: improve comments in npm rules (#1649)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan committed Apr 16, 2024
1 parent dd7354d commit 498875a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 14 deletions.
2 changes: 1 addition & 1 deletion examples/npm_deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ js_test(
)

#######################################
# Case 7: use a first-party npm package from within our Bazel monorepo workspace
# Case 7: use a first-party npm package within our Bazel monorepo workspace from an npm_package target

write_file(
name = "write7",
Expand Down
2 changes: 1 addition & 1 deletion npm/private/npm_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ def _npm_import_links_rule_impl(rctx):
if transitive_closure_pattern:
# transitive closure deps pattern is used for breaking circular deps;
# this pattern is used to break circular dependencies between 3rd
# party npm deps; it is not recommended for 1st party deps
# party npm deps; it is not used for 1st party deps
for (dep_name, dep_versions) in rctx.attr.transitive_closure.items():
for dep_version in dep_versions:
store_package, store_version = utils.parse_pnpm_package_key(dep_name, dep_version)
Expand Down
25 changes: 13 additions & 12 deletions npm/private/npm_package_store.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
# this is a ref npm_link_package, a downstream terminal npm_link_package
# for this npm dependency will create the dep symlinks for this dep;
# this pattern is used to break circular dependencies between 3rd
# party npm deps; it is not recommended for 1st party deps
# party npm deps; it is not used for 1st party deps
direct_ref_deps[dep] = dep_aliases

for store in ctx.attr.src[NpmPackageInfo].npm_package_store_infos.to_list():
Expand All @@ -254,9 +254,10 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
files.append(utils.make_symlink(ctx, dep_symlink_path, dep_package_store_directory.path))
npm_package_store_infos.append(store)
else:
# if ctx.attr.src is _not_ set then this is a terminal 3p package with ctx.attr.deps is
# being the transitive closure of deps; this pattern is used to break circular dependencies
# between 3rd party npm deps; it is not recommended for 1st party deps
# ctx.attr.src can be unspecified when the rule is a npm_package_store_internal; when it is _not_
# set, this is a terminal 3p package with ctx.attr.deps being the transitive closure of
# deps; this pattern is used to break circular dependencies between 3rd party npm deps; it
# is not used for 1st party deps
deps_map = {}
for dep, _dep_aliases in ctx.attr.deps.items():
dep_package = dep[NpmPackageStoreInfo].package
Expand All @@ -268,7 +269,7 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
else:
# this is a ref npm_link_package, a downstream terminal npm_link_package for this npm
# depedency will create the dep symlinks for this dep; this pattern is used to break
# for lifecycle hooks on 3rd party deps; it is not recommended for 1st party deps
# for lifecycle hooks on 3rd party deps; it is not used for 1st party deps
direct_ref_deps[dep] = dep_aliases
for dep in ctx.attr.deps:
dep_package_store_name = utils.package_store_name(dep[NpmPackageStoreInfo].package, dep[NpmPackageStoreInfo].version)
Expand Down Expand Up @@ -308,13 +309,13 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
for npm_package_store in npm_package_store_infos
])
else:
# if ctx.attr.src is _not_ set then this is a terminal 3p package with ctx.attr.deps is
# being the transitive closure of deps; this pattern is used to break circular dependencies
# between 3rd party npm deps; it is not recommended for 1st party deps because
# npm_package_store_infos is the transitive closure of all the entire package store deps, we
# can safely add just `files` from each of these to `transitive_files_depset`. Doing so
# reduces the size of `transitive_files_depset` significantly and reduces analysis time and
# Bazel memory usage during analysis
# ctx.attr.src can be unspecified when the rule is a npm_package_store_internal; when ctx.attr.src is
# _not_ set, this is a terminal 3p package with ctx.attr.deps being the transitive closure
# of deps; this pattern is used to break circular dependencies between 3rd party npm deps;
# it is not used for 1st party deps; because npm_package_store_infos is the transitive
# closure of all the entire package store deps, we can safely add just `files` from each of
# these to `transitive_files_depset`; doing so reduces the size of `transitive_files_depset`
# significantly and reduces analysis time and Bazel memory usage during analysis
transitive_files_depset = depset(files, transitive = [
npm_package_store.files
for npm_package_store in npm_package_store_infos
Expand Down

0 comments on commit 498875a

Please sign in to comment.