https resolutions in package.json causes failures

[paullewis]

If I have the following package.json:

{
  "name": "repro",
  "version": "1.0.0",
  "resolutions": {
    "jsonify": "http://localhost/jsonify-1.0.0.tgz"
  },
  "dependencies": {
    "json-stable-stringify": "1.0.1"
  }
}

The build will fail with this error:

ERROR: BUILD.bazel:13:22: in deps attribute of npm_package_store_internal rule //:.aspect_rules_js/node_modules/json-stable-stringify/1.0.1/pkg: target '//:.aspect_rules_js/node_modules/jsonify/@localhost/jsonify-1.0.0.tgz/ref' does not exist. Since this rule was created by the macro 'npm_link_all_packages', the error might have been caused by the macro implementation

(Here I'm mimicking an npm registry with localhost but it applies no matter what the origin is.)

AFAICT there's an issue with the paths in external/npm__json-stable-stringify__1.0.1__links, and in particular the ref_deps doesn't seem to account for the fact we've got a tarball:

ref_deps = {
    ":.aspect_rules_js/{}/jsonify/@localhost/jsonify-1.0.0.tgz/ref".format(link_root_name): "jsonify",
}

[gregmagolan]

I took a minute to try to repo and besides another issue with # not being escaped, I'm able to specify http:// versions and pnpm overrides (should be equivalent to resolutions). #424

What does the pnpm lockfile entry look like for jsonify for you?

[paullewis]

The whole pnpm lock file is:

lockfileVersion: 5.4

overrides:
  jsonify: http://localhost/jsonify-1.0.0.tgz

specifiers:
  json-stable-stringify: 1.0.1

dependencies:
  json-stable-stringify: 1.0.1

packages:

  /json-stable-stringify/1.0.1:
    resolution: {integrity: sha1-mnWdOcXy/1A/1TAGRu1EX4jE+a8=, tarball: https://github.com/substack/json-stable-stringify/archive/refs/tags/1.0.1.tar.gz}
    dependencies:
      jsonify: '@localhost/jsonify-1.0.0.tgz'
    dev: false

  '@localhost/jsonify-1.0.0.tgz':
    resolution: {tarball: http://localhost/jsonify-1.0.0.tgz}
    name: 'foo/jsonify'
    version: 1.0.0
    dev: false

[gregmagolan]

Interesting, that is almost equivalent to what I got in #424

  '@github.com/aspect-build/test-packages/releases/download/0.0.0/jsonify-0.0.0.tgz':
    resolution: {tarball: https://github.com/aspect-build/test-packages/releases/download/0.0.0/jsonify-0.0.0.tgz}
    name: jsonify
    version: 0.0.0
    dev: true

  /json-stable-stringify/1.0.1:
    resolution: {integrity: sha512-i/J297TW6xyj7sDFa7AmBPkQvLIxWr2kKPWI26tXydnZrzVAocNqn5DMNT1Mzk0vit1V5UkRM7C1KdVNp7Lmcg==}
    dependencies:
      jsonify: '@github.com/aspect-build/test-packages/releases/download/0.0.0/jsonify-0.0.0.tgz'
    dev: true

minus the foo/jsonify name. How does the foo sneak in?

[paullewis]

I believe it comes from the package.json in the tarball. In this case I resolve to a version of jsonify which is different from the npm-published version and carries a slightly different name in the package.json.

[gregmagolan]

Ahhh. That would do it. When I manually add a foo/ to the lockfile there I can reproduce the issue.

I'm not sure where it goes wrong but I'll track it down since this case should work if it works outside of bazel.

[paullewis]

👍🏻 yeah just confirmed that pnpm definitely looks in the tarball for the package.json and the name inside. If it has a scoped name, e.g., @foo/jsonify, then that's what it drops in the pnpm lock file.

[gregmagolan]

Mostly a trivial fix but definitely would have been a hard one for someone not familiar with the linking code. #424 resolves it. The crux of the issue is that the package name was getting baked into the virtual store target name and in this special case it should not have been.

[paullewis]

Excellent - thank you!