Skip to content

Commit cb28aab

Browse files
authored
Prepare for fixing double-parsing (#2477)
Currently we parse the lockfile in both the extension and the repo rule (so we can generate all the bzl/BUILD file). The plan to fix that is to move the file generation into the module extension and reuse the parse state. To do that, we must decouple the generation from the side effects (dumping files to disk). While here, I changed the args to `generate` to enforce it becomes pure. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases
1 parent 765917a commit cb28aab

File tree

2 files changed

+28
-23
lines changed

2 files changed

+28
-23
lines changed

npm/private/npm_translate_lock.bzl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,9 @@ See https://github.com/aspect-build/rules_js/issues/1445
155155

156156
rctx.report_progress("Generating starlark for npm dependencies")
157157

158-
generate_repository_files(
159-
rctx,
158+
files = generate_repository_files(
159+
rctx.name,
160+
rctx.attr,
160161
importers,
161162
packages,
162163
state.patched_dependencies(),
@@ -167,6 +168,9 @@ See https://github.com/aspect-build/rules_js/issues/1445
167168
state.npm_auth(),
168169
)
169170

171+
for filename, contents in files.items():
172+
rctx.file(filename, contents)
173+
170174
# Support bazel <v8.3 by returning None if repo_metadata is not defined
171175
if not hasattr(rctx, "repo_metadata"):
172176
return None

npm/private/npm_translate_lock_generate.bzl

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,13 @@ _PACKAGE_JSON_BZL_FILENAME = "package_json.bzl"
5050
_RESOLVED_JSON_FILENAME = "resolved.json"
5151

5252
# buildifier: disable=function-docstring
53-
def generate_repository_files(rctx, importers, packages, patched_dependencies, only_built_dependencies, root_package, default_registry, npm_registries, npm_auth):
53+
def generate_repository_files(rctx_name, attr, importers, packages, patched_dependencies, only_built_dependencies, root_package, default_registry, npm_registries, npm_auth):
5454
# empty line after bzl docstring since buildifier expects this if this file is vendored in
55-
generated_by_prefix = "\"\"\"@generated by npm_translate_lock(name = \"{}\", pnpm_lock = \"{}\")\"\"\"\n".format(helpers.to_apparent_repo_name(rctx.name), str(rctx.attr.pnpm_lock))
55+
generated_by_prefix = "\"\"\"@generated by npm_translate_lock(name = \"{}\", pnpm_lock = \"{}\")\"\"\"\n".format(helpers.to_apparent_repo_name(rctx_name), str(attr.pnpm_lock))
5656

57-
npm_imports = helpers.get_npm_imports(importers, packages, rctx.attr.replace_packages, patched_dependencies, only_built_dependencies, root_package, rctx.name, rctx.attr, rctx.attr.lifecycle_hooks, rctx.attr.lifecycle_hooks_execution_requirements, rctx.attr.lifecycle_hooks_use_default_shell_env, npm_registries, default_registry, npm_auth)
57+
npm_imports = helpers.get_npm_imports(importers, packages, attr.replace_packages, patched_dependencies, only_built_dependencies, root_package, rctx_name, attr, attr.lifecycle_hooks, attr.lifecycle_hooks_execution_requirements, attr.lifecycle_hooks_use_default_shell_env, npm_registries, default_registry, npm_auth)
58+
59+
final_rctx_files = {}
5860

5961
rctx_files = {
6062
"BUILD.bazel": [
@@ -186,10 +188,10 @@ js_binary(name = "sync", entry_point = "noop.js")
186188
)
187189

188190
# Generate visibility configuration first to check if it's actually non-empty
189-
npm_visibility_config_generated = _generate_npm_visibility_config(rctx.attr.package_visibility)
191+
npm_visibility_config_generated = _generate_npm_visibility_config(attr.package_visibility)
190192

191193
# Check if the generated visibility config is actually non-empty
192-
has_package_visibility = rctx.attr.package_visibility != None and npm_visibility_config_generated != None
194+
has_package_visibility = attr.package_visibility != None and npm_visibility_config_generated != None
193195

194196
# Only keep the generated config if it's non-empty
195197
npm_visibility_config = npm_visibility_config_generated if has_package_visibility else None
@@ -223,10 +225,10 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
223225
fail(msg)
224226
{validation_call}
225227
""".format(
226-
defs_bzl_file = "@{}//:{}".format(rctx.name, _DEFS_BZL_FILENAME),
228+
defs_bzl_file = "@{}//:{}".format(rctx_name, _DEFS_BZL_FILENAME),
227229
link_packages_comma_separated = "\"'\" + \"', '\".join(_IMPORTER_PACKAGES) + \"'\"" if package_to_importer else "\"\"",
228230
root_package = root_package,
229-
pnpm_lock_label = rctx.attr.pnpm_lock,
231+
pnpm_lock_label = attr.pnpm_lock,
230232
validation_call = validation_call,
231233
),
232234
]
@@ -311,17 +313,17 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
311313
resolved_json_rel_path = "{}/{}".format(link_alias, _RESOLVED_JSON_FILENAME)
312314
resolved_json_file_path = "{}/{}".format(link_package, resolved_json_rel_path) if link_package else resolved_json_rel_path
313315

314-
rctx.file(resolved_json_file_path, json.encode({
316+
final_rctx_files[resolved_json_file_path] = json.encode({
315317
# Allow consumers to auto-detect this filetype
316318
"$schema": "https://docs.aspect.build/rules/aspect_rules_js/docs/npm_translate_lock",
317319
"version": _import.version,
318320
"integrity": _import.integrity,
319-
}))
321+
})
320322
rctx_files[build_file].append("exports_files([\"{}\"])".format(resolved_json_rel_path))
321323

322324
# the package_json.bzl for this package
323325
if _import.package_info["has_bin"]:
324-
if rctx.attr.generate_bzl_library_targets:
326+
if attr.generate_bzl_library_targets:
325327
rctx_files[build_file].append("""load("@bazel_skylib//:bzl_library.bzl", "bzl_library")""")
326328

327329
for link_alias in link_aliases:
@@ -342,12 +344,9 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
342344
link_package = link_package,
343345
package_json_bzl = _PACKAGE_JSON_BZL_FILENAME,
344346
)
345-
rctx.file(
346-
package_json_bzl_file_path,
347-
_BIN_TMPL.format(
348-
repo_package_json_bzl = repo_package_json_bzl,
349-
),
350-
)
347+
rctx_files[package_json_bzl_file_path] = [_BIN_TMPL.format(
348+
repo_package_json_bzl = repo_package_json_bzl,
349+
)]
351350

352351
# Generate the first-party package stores and linking of first-party packages
353352
for i, fp_link in enumerate(fp_links.values()):
@@ -357,7 +356,7 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
357356
fp_deps = fp_link["deps"]
358357
fp_target = "//{}:{}".format(
359358
fp_path,
360-
rctx.attr.npm_package_target_name.replace("{dirname}", paths.basename(fp_path)),
359+
attr.npm_package_target_name.replace("{dirname}", paths.basename(fp_path)),
361360
)
362361
fp_package_store_name = utils.package_store_name(fp_package, fp_version)
363362

@@ -390,7 +389,7 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
390389

391390
# Generate a single _FP_DIRECT_TMPL block with all link packages
392391
if fp_link["link_packages"]:
393-
package_visibility, _ = helpers.gather_values_from_matching_names(True, rctx.attr.package_visibility, "*", fp_package)
392+
package_visibility, _ = helpers.gather_values_from_matching_names(True, attr.package_visibility, "*", fp_package)
394393
if not package_visibility:
395394
package_visibility = None
396395

@@ -521,7 +520,7 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
521520

522521
rctx_files[_DEFS_BZL_FILENAME] = defs_bzl_contents
523522

524-
for filename, contents in rctx.attr.additional_file_contents.items():
523+
for filename, contents in attr.additional_file_contents.items():
525524
if not rctx_files.get(filename, False):
526525
rctx_files[filename] = contents
527526
elif filename.endswith(".bzl"):
@@ -537,8 +536,10 @@ def npm_link_all_packages(name = "node_modules", imported_links = [], prod = Tru
537536
else:
538537
rctx_files[filename].extend(contents)
539538

540-
for filename, contents in rctx_files.items():
541-
rctx.file(filename, generated_by_prefix + "\n" + "\n".join(contents).rstrip() + "\n")
539+
return final_rctx_files | {
540+
filename: generated_by_prefix + "\n" + "\n".join(contents).rstrip() + "\n"
541+
for filename, contents in rctx_files.items()
542+
}
542543

543544
def _generate_npm_link_targets(links_targets):
544545
"""Generate the npm_link_targets() macro given the links_targets struct"""

0 commit comments

Comments
 (0)