javascript: Add an explicit npm_distribution target instead of coup…

…ling to `node_package` (pantsbuild#18925)

The coupling of `node_package` (generated from the `package_json`
target) with the `package` goal caused some unwanted side effects when
trying to integrate with other backends, `docker` to be exact.

Because of the way the dependency graph is constructed between in-repo
nodejs packages, this caused package:able targets to always be included.
See this bit of code:
https://github.com/pantsbuild/pants/blob/f95ed94d13a8531a7fdab8318d9f597b08d04371/src/python/pants/backend/docker/util_rules/docker_build_context.py#L280
The transitive dependencies of the `docker_image` currently causes
packaging of workspace packages, even though you probably do not want to
include the resulting tarballs at all.

Even worse, some package manager + workspace combinations are flat out
broken, see pnpm/pnpm#4351.

This solution "moves" the package:able target out of the dependency
graph by placing it as standalone thing, outside of the automagically
constructed dependencies.


Fixes pantsbuild#18922
This PR does not provide a work-around for the linked PNPM issue.

src/python/pants/backend/javascript/install_node_package.py

@@ -30,6 +30,7 @@
 from pants.engine.rules import Rule, collect_rules, rule
 from pants.engine.target import (
     SourcesField,
+    Target,
     TransitiveTargets,
     TransitiveTargetsRequest,
     targets_with_sources_types,
@@ -54,6 +55,10 @@ def project_dir(self) -> str:
     def join_relative_workspace_directory(self, path: str) -> str:
         return os.path.join(self.project_env.relative_workspace_directory(), path)
 
+    @property
+    def target(self) -> Target:
+        return self.project_env.ensure_target()
+
 
 @dataclass(frozen=True)
 class InstalledNodePackageWithSource(InstalledNodePackage):
@@ -127,7 +132,9 @@ async def add_sources_to_installed_node_package(
     req: InstalledNodePackageRequest,
 ) -> InstalledNodePackageWithSource:
     installation = await Get(InstalledNodePackage, InstalledNodePackageRequest, req)
-    transitive_tgts = await Get(TransitiveTargets, TransitiveTargetsRequest([req.address]))
+    transitive_tgts = await Get(
+        TransitiveTargets, TransitiveTargetsRequest([installation.target.address])
+    )
 
     source_files = await _get_relevant_source_files(
         (tgt[SourcesField] for tgt in transitive_tgts.dependencies if tgt.has_field(SourcesField)),

src/python/pants/backend/javascript/package/rules.py

@@ -21,6 +21,7 @@
     NodeBuildScriptSourcesField,
     NodePackageNameField,
     NodePackageVersionField,
+    NPMDistributionTarget,
     PackageJsonSourceField,
 )
 from pants.build_graph.address import Address
@@ -44,10 +45,9 @@
 
 @dataclass(frozen=True)
 class NodePackageTarFieldSet(PackageFieldSet):
-    required_fields = (PackageJsonSourceField, NodePackageNameField, NodePackageVersionField)
+    required_fields = (PackageJsonSourceField, OutputPathField)
     source: PackageJsonSourceField
-    name: NodePackageNameField
-    version: NodePackageVersionField
+    output_path: OutputPathField
 
 
 @dataclass(frozen=True)
@@ -83,22 +83,33 @@ async def pack_node_package_into_tgz_for_publication(
     installation = await Get(
         InstalledNodePackageWithSource, InstalledNodePackageRequest(field_set.address)
     )
-    archive_file = installation.project_env.project.pack_archive_format.format(
-        field_set.name.value, field_set.version.value
-    )
+    node_package = installation.project_env.ensure_target()
+    name = node_package.get(NodePackageNameField).value
+    version = node_package.get(NodePackageVersionField).value
+    if version is None:
+        raise ValueError(
+            f"{field_set.source.file_path}#version must be set in order to package a {NPMDistributionTarget.alias}."
+        )
+    archive_file = installation.project_env.project.pack_archive_format.format(name, version)
     result = await Get(
         ProcessResult,
         NodeJsProjectEnvironmentProcess(
             installation.project_env,
             args=("pack",),
-            description=f"Packaging .tgz archive for {field_set.name.value}@{field_set.version.value}",
+            description=f"Packaging .tgz archive for {name}@{version}",
             input_digest=installation.digest,
             output_files=(installation.join_relative_workspace_directory(archive_file),),
             level=LogLevel.INFO,
         ),
     )
+    if field_set.output_path.value:
+        digest = await Get(Digest, AddPrefix(result.output_digest, field_set.output_path.value))
+    else:
+        digest = result.output_digest
 
-    return BuiltPackage(result.output_digest, (BuiltPackageArtifact(archive_file),))
+    return BuiltPackage(
+        digest, (BuiltPackageArtifact(archive_file, tuple(result.stderr.decode().splitlines())),)
+    )
 
 
 _NOT_ALPHANUMERIC = re.compile("[^0-9a-zA-Z]+")

src/python/pants/backend/javascript/package/rules_test.py

@@ -17,6 +17,7 @@
     NodePackageTarFieldSet,
 )
 from pants.backend.javascript.package.rules import rules as package_rules
+from pants.backend.javascript.package_json import NPMDistributionTarget
 from pants.backend.javascript.target_types import JSSourcesGeneratorTarget, JSSourceTarget
 from pants.build_graph.address import Address
 from pants.core.goals.package import BuiltPackage
@@ -45,6 +46,7 @@ def rule_runner(package_manager: str) -> RuleRunner:
             *package_json.target_types(),
             JSSourceTarget,
             JSSourcesGeneratorTarget,
+            NPMDistributionTarget,
             FilesGeneratorTarget,
         ],
         objects=dict(package_json.build_file_aliases().objects),
@@ -60,6 +62,8 @@ def test_creates_tar_for_package_json(rule_runner: RuleRunner, package_manager:
                 """\
                 package_json(dependencies=[":readme"])
                 files(name="readme", sources=["*.md"])
+
+                npm_distribution(name="ham-dist")
                 """
             ),
             "src/js/package.json": json.dumps(
@@ -83,7 +87,7 @@ def test_creates_tar_for_package_json(rule_runner: RuleRunner, package_manager:
             "src/js/lib/index.mjs": "",
         }
     )
-    tgt = rule_runner.get_target(Address("src/js", generated_name="ham"))
+    tgt = rule_runner.get_target(Address("src/js", target_name="ham-dist"))
     result = rule_runner.request(BuiltPackage, [NodePackageTarFieldSet.create(tgt)])
     rule_runner.write_digest(result.digest)
 

src/python/pants/backend/javascript/package_json.py

@@ -254,8 +254,8 @@ class NodePackageVersionField(StringField):
         This field should not be overridden; use the value from target generation.
         """
     )
-    required = True
-    value: str
+    required = False
+    value: str | None
 
 
 class NodeThirdPartyPackageVersionField(NodePackageVersionField):
@@ -320,6 +320,25 @@ class NodePackageTarget(Target):
     )
 
 
+class NPMDistributionTarget(Target):
+    alias = "npm_distribution"
+
+    help = help_text(
+        """
+        A publishable npm registry distribution, typically a gzipped tarball
+        of the sources and any resources, but omitting the lockfile.
+
+        Generated using the projects package manager `pack` implementation.
+        """
+    )
+
+    core_fields = (
+        *COMMON_TARGET_FIELDS,
+        PackageJsonSourceField,
+        OutputPathField,
+    )
+
+
 class PackageJsonTarget(TargetGenerator):
     alias = "package_json"
     core_fields = (
@@ -595,7 +614,7 @@ def from_package_json(cls, pkg_json: PackageJson) -> PackageJsonScripts:
 class PackageJson:
     content: FrozenDict[str, Any]
     name: str
-    version: str
+    version: str | None
     snapshot: Snapshot
     workspaces: tuple[str, ...] = ()
     module: Literal["commonjs", "module"] | None = None
@@ -671,7 +690,11 @@ async def find_owning_package(request: OwningNodePackageRequest) -> OwningNodePa
         ),
     )
     package_json_tgts = sorted(
-        (tgt for tgt in candidate_targets if tgt.has_field(PackageJsonSourceField)),
+        (
+            tgt
+            for tgt in candidate_targets
+            if tgt.has_field(PackageJsonSourceField) and tgt.has_field(NodePackageNameField)
+        ),
         key=lambda tgt: tgt.address.spec_path,
         reverse=True,
     )
@@ -690,7 +713,7 @@ async def parse_package_json(content: FileContent) -> PackageJson:
     return PackageJson(
         content=parsed_package_json,
         name=parsed_package_json["name"],
-        version=parsed_package_json["version"],
+        version=parsed_package_json.get("version"),
         snapshot=await Get(Snapshot, PathGlobs([content.path])),
         module=parsed_package_json.get("type"),
         workspaces=tuple(parsed_package_json.get("workspaces", ())),

src/python/pants/backend/javascript/resolve.py

@@ -9,6 +9,7 @@
 from pants.backend.javascript import nodejs_project
 from pants.backend.javascript.nodejs_project import AllNodeJSProjects, NodeJSProject
 from pants.backend.javascript.package_json import (
+    NodePackageNameField,
     OwningNodePackage,
     OwningNodePackageRequest,
     PackageJsonSourceField,
@@ -48,7 +49,9 @@ async def _get_node_package_json_directory(req: RequestNodeResolve) -> str:
         WrappedTargetRequest(req.address, description_of_origin="the `ChosenNodeResolve` rule"),
     )
     target: Target | None
-    if wrapped.target.has_field(PackageJsonSourceField):
+    if wrapped.target.has_field(PackageJsonSourceField) and wrapped.target.has_field(
+        NodePackageNameField
+    ):
         target = wrapped.target
     else:
         owning_pkg = await Get(OwningNodePackage, OwningNodePackageRequest(wrapped.target.address))