[Bug]: Failures caused by duplicate keys in transitive package.json files

[UebelAndre]

What happened?

I'm getting an error from a transitive dependency browserify-zlib

INFO: repository @npm__browserify-zlib__0.1.4' used the following cache hits instead of downloading the corresponding file.
 * Hash 'd7d384a6aeef5a0b07e9692f9012500c5bc94b5b8f71b14e438bfd094f37f5d3be6595d464ee97a44ea1342aaf948223fb87d9bd1889e83b00437f361b08b24d' for https://registry.npmjs.org/browserify-zlib/-/browserify-zlib-0.1.4.tgz
If the definition of 'repository @npm__browserify-zlib__0.1.4' was updated, verify that the hashes were also updated.
ERROR: An error occurred during the fetch of repository 'npm__browserify-zlib__0.1.4':
   Traceback (most recent call last):
	File "/private/var/tmp/_bazel_user/93186f6054b5156b064858613bb6d5db/external/aspect_rules_js/npm/private/npm_import.bzl", line 476, column 27, in _impl
		pkg_json = json.decode(rctx.read(pkg_json_path))
Error in decode: at offset 685, object has duplicate key: "main"
ERROR: /Users/user/Code/example/WORKSPACE.bazel:164:17: fetching _npm_import rule //external:npm__browserify-zlib__0.1.4: Traceback (most recent call last):
	File "/private/var/tmp/_bazel_user/93186f6054b5156b064858613bb6d5db/external/aspect_rules_js/npm/private/npm_import.bzl", line 476, column 27, in _impl
		pkg_json = json.decode(rctx.read(pkg_json_path))
Error in decode: at offset 685, object has duplicate key: "main"
ERROR: /private/var/tmp/_bazel_user/93186f6054b5156b064858613bb6d5db/external/npm/BUILD.bazel:3258:6: @npm//:npm__browserify-zlib__0.1.4_source_directory depends on @npm__browserify-zlib__0.1.4//:source_directory in repository @npm__browserify-zlib__0.1.4 which failed to fetch. no such package '@npm__browserify-zlib__0.1.4//': at offset 685, object has duplicate key: "main"

The package.json of this package looks like the following

{
  "name": "browserify-zlib",
  "version": "0.1.4",
  "description": "Full zlib module for browserify",
  "keywords": ["zlib", "browserify"],
  "main": "index.js",
  "directories": {
    "test": "test"
  },
  "dependencies": {
    "pako": "~0.2.0"
  },
  "devDependencies": {
    "tape": "^2.12.3",
    "brfs": "^1.0.1"
  },
  "testling": {
    "files": "test/*.js",
    "browsers": [
      "ie/6..latest",
      "chrome/22..latest",
      "firefox/16..latest",
      "safari/latest",
      "opera/11.0..latest",
      "iphone/6",
      "ipad/6",
      "android-browser/latest"
    ]
  },
  "scripts": {
    "test": "node_modules/tape/bin/tape test/*.js"
  },
  "main": "src/index.js",
  "author": "Devon Govett <[email protected]>",
  "license": "MIT",
  "repository": {
    "type": "git",
    "url": "git://github.com/devongovett/browserify-zlib.git"
  }
}

Is this something rules_js should be resilient to? I get that some parts of package.json are pretty critical but this is clearly something that's existed for a while and likely hasn't impacted folks outside of Bazel.

Version

Development (host) and target OS/architectures:

Output of bazel --version: 6.1.0

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:
rules_js-1.24.1

Language(s) and/or frameworks involved:

How to reproduce

Create a workspace with a dependency on browserify-zlib =0.14.0

Any other information?

No response

[jbedard]

The bazel json.decode is used and we can't really control how strict that is, but you can patch the bad package using pnpm patches or npm_translate_lock patches.

See the npm_translate_lock_subdir_patch example test.

[UebelAndre]

The bazel json.decode is used and we can't really control how strict that is

I guess I'm suggesting that it's insufficient for the uses here and some other json parser might be needed. That said, this json is obviously broken and I don't really know how wide spread problems like this are so may not be a big issue? But I do think seeing this failure puts unnecessary burden on Bazel users that could be solved for within this repo.

[jbedard]

While it is a hassle for users, fixing bazel issues in every ruleset is also a hassle :/

It looks like this is actually fixed in bazel now though, which is the best solution: bazelbuild/bazel#15605