(JavaScript) Throw a more descriptive error when the package name is …

…missing (#21159)

Closes #20859.

While this is a simple fix and addresses the linked issue, there is an
interesting edge case to discuss:

Internal packages do not necessarily have a name field in `package.json`
(see wireapp/wire-desktop#1692,
facebook/react#13107 for examples). The
JavaScript backend in Pants does require that each package.json define a
name, but I'm not so sure that's necessarily the right behavior.

It's worth considering whether we should make names optional in Pants,
given that larger JavaScript monorepos may have internal packages that
are not meant to be published. Furthermore, different JS package
managers handle this situation differently -

* [Bun doesn't handle package.json files without a
name](oven-sh/bun#6317)
* [npm assigns the parent directory name as the
name](npm/cli#2264)

docs/notes/2.23.x.md

@@ -123,15 +123,16 @@ pants to invoke `corepack`s default versions of the package managers instead.
 (npm, pnpm, yarn) without providing a corresponding `[nodejs].package_managers` version setting. The version is then
 entirely handled by `corepack`. Previously this mode caused pants to fail.
 
-The internal installation mechanism for node_modules has changed. 
-Previously, Pants installed each package separately in sandboxes and merged the results, creating a node_modules for all dependent packages in the workspace. 
+The internal installation mechanism for node_modules has changed.
+Previously, Pants installed each package separately in sandboxes and merged the results, creating a node_modules for all dependent packages in the workspace.
 Now, this is delegated to the package managers, using each package manager's support for workspaces.
 
 This fixes an issue with integrity file collisions when newer versions of package managers (e.g. the [hidden lockfiles](https://docs.npmjs.com/cli/v9/configuring-npm/package-lock-json#hidden-lockfiles) introduced in npm v7).
 
 `pants export --resolve=<js-resolve>` now has basic support for exporting the package manager installation artifacts
 including `node_modules`. This can be used to inspect the installation, or to enable IDE:s to discover the packages.
 
+Pants will output a more helpful error message if there is no `name` field defined in the `package.json` file, or if the `name` field is empty.
 
 #### Shell
 

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

@@ -683,9 +683,13 @@ async def find_owning_package(request: OwningNodePackageRequest) -> OwningNodePa
 @rule
 async def parse_package_json(content: FileContent) -> PackageJson:
     parsed_package_json = FrozenDict.deep_freeze(json.loads(content.content))
+    package_name = parsed_package_json.get("name")
+    if not package_name:
+        raise ValueError("No package name found in package.json")
+
     return PackageJson(
         content=parsed_package_json,
-        name=parsed_package_json["name"],
+        name=package_name,
         version=parsed_package_json.get("version"),
         snapshot=await Get(Snapshot, PathGlobs([content.path])),
         module=parsed_package_json.get("type"),

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

@@ -27,7 +27,7 @@
 from pants.engine.internals.scheduler import ExecutionError
 from pants.engine.rules import QueryRule
 from pants.engine.target import AllTargets
-from pants.testutil.rule_runner import RuleRunner
+from pants.testutil.rule_runner import RuleRunner, engine_error
 from pants.util.frozendict import FrozenDict
 
 
@@ -86,6 +86,18 @@ def test_parses_package_jsons(rule_runner: RuleRunner) -> None:
     }
 
 
+def test_parse_package_json_without_name(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "src/js/BUILD": "package_json()",
+            # No name in package.json, should cause an error.
+            "src/js/package.json": json.dumps({"version": "0.0.1"}),
+        }
+    )
+    with engine_error(ValueError, contains="No package name found in package.json"):
+        rule_runner.request(AllPackageJson, [])
+
+
 def test_generates_third_party_node_package_targets(rule_runner: RuleRunner) -> None:
     rule_runner.write_files(
         {