[Bug]: "Cannot find module" error for dependency that is a peerDep of a first-party package

[etlovett]

What happened?

We have two rules_js-based libraries; let's call them leaf and consumer. consumer depends on leaf, and each have a set of third-party dependencies. Applications might choose to depend on either or both. Our expectation is that we should be able to do the following:

// leaf/package.json
{
  "peerDependencies": {
    "third-party-dep-1": "whatever version",
    "third-party-dep-2": "whatever version",
  },
  "devDependencies": {
    "third-party-dep-1": "whatever version",
    "third-party-dep-2": "whatever version",
  }
}

// consumer/package.json
{
  "peerDependencies": {
    "@first-party/leaf": "workspace:*",
    "third-party-dep-1": "whatever version",
    "third-party-dep-2": "whatever version",
  },
  "devDependencies": {
    "@first-party/leaf": "workspace:*",
    "third-party-dep-1": "whatever version",
    "third-party-dep-2": "whatever version",
  }
}

// some-app/package.json
{
  "dependencies": {
    "@first-party/consumer": "workspace:*",
    "@first-party/leaf": "workspace:*", // optional
    "third-party-dep-1": "whatever version",
    "third-party-dep-2": "whatever version",
  }
}

I’m trying to set up just the two libraries right now, using essentially the package.json files above. However, when I do all the pnpm installs and then run the Jest tests in consumer (bazel test //path/to/consumer/src:test), I get the following two errors in the output:

Cannot find module 'third-party-dep-1' from '../../../../node_modules/.aspect_rules_js/@[email protected]/node_modules/@first-party/leaf/src/some-file.js'
Cannot find module 'third-party-dep-2' from '../../../../../../../../../../../../../../../../../../execroot/rh/bazel-out/darwin_arm64-fastbuild/bin/bazel/js/jest/config/src/jest-env.mjs'

Changing all of consumer’s peerDeps/devDeps to (regular) deps doesn’t fix anything. Changing all of leaf’s peerDeps to (regular) deps does fix it, since then it can find its own dependencies, but this doesn’t seem to us like the right philosophical approach for a monorepo.

It also seems odd that utils has itself in its own node_modules, and we’re not sure why that would be.

Are the first-party package's peerDeps not getting properly passed along the Bazel graph?

Version

Development (host) and target OS/architectures: Running on macOS

Output of bazel --version:bazel 6.2.0

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file: We're running off commit 8dc6c8be10a44900408c26935b6d7182253a0b2f.

Language(s) and/or frameworks involved: TS and Jest

How to reproduce

No response

Any other information?

No response

[Aghassi]

In our case (I was looking through our lockfile) peer dependencies are not ending up encoded in the leaf package. Dev Dependencies are. A common pattern in the JS ecosystem is to declare things as peer that you expect an app to declare when they use your package and then redeclare your deps as developer deps so your tools don't yell at you. This avoids nested structures because very rarely can libraries dictate what versions apps end up with. The reason declaring as dependencies fixes it is because our lockfile then encodes the dependencies.

What's odd though is that some of our 3rd party dependencies specify peerDependencies in the lockfile. It looks like workspace packages don't encode peer dependencies. Only dev dependencies and actual dependencies.

Some helpful links potentially from searching

• pnpm install workspace package unmet/missing peerDependencies warning pnpm/pnpm#4843
• Injected dependencies are not recopied after a workspace project is rebuilt pnpm/pnpm#4407

[Aghassi]

One thing I wanted to add to this is that we set the rootDir of jest to be the bazel target directory instead of the root of the bazel workspace. This makes importing folder paths easier. However, I think (and will have to validate) one of the problems might be that this omits the root of the pnpm workspace from the "seeable" areas that jest knows to look for. So when we run, we get visibility to any dependencies of the current package, but not the whole workspace. I will try adjusting this when I'm in office next week and see if it solves anything.

If not, I plan to make a repro here.

[sjbarag]

There seems to be something fishy with a first-party dependency declaring both a peerDependency and devDependency (leaf in the example at the top). Having one or the other seems to work well though.

I got a repro up in https://github.com/sjbarag/rules_js-peer-and-dev-dep-ignored , which uses only rules_js. So we can safely eliminate Jest/rules_js as part of the issue, but I haven't found a proper solution yet.

[sjbarag]

Ah, okay so there's a workaround using pnpm.packageExtensions that seems safe. It's unfortunate that it's required, but it should help with any first-party dependencies that have peer+dev dependencies.

1. Remove the devDependencies entry in the leaf-node package.json:

// packages/leaf/package.json
{
  "name": "leaf",
  "version": "1.2.3-dunno",
  "devDependencies": {
    "is-odd": "*",
-   "left-pad": "^1.3.0"
  },
  "peerDependencies": {
    "left-pad": "^1.3.0"
  }
}

2. Re-add the devDependencies entry via pnpm.packageExtensions at the workspace-level package.json:

// package.json
{
  "name": "some-workspace",
  "version": "0.0.1-workspace-package",
+ "pnpm": {
+   "packageExtensions": {
+     "leaf@*": {
+       "devDependencies": {
+         "left-pad": "^1.3.0"
+       }
+     }
+   }
  }
}

3. Re-run pnpm install to update the lockfile

left-pad (or whatever the dev+peer dependency is) will still be declared a peer dependency for all consumers — even if they install through a public registry and with npm or yarn — and local development should still get left-pad installed via pnpm install since pnpm is injecting that declaration.

Hope this helps 😃

[alexeagle]

@etlovett is this issue resolved by @sjbarag comment there? Is there still a bug in rules_js?

[jfirebaugh]

I believe there still is a bug in rules_js. I made my own minimal repro: https://github.com/jfirebaugh/rules_js-1p-peer-dep.

[etlovett]

@etlovett is this issue resolved by @sjbarag comment there? Is there still a bug in rules_js?

@alexeagle Sorry I missed your comment. I haven't tried that workaround, but FWIW I'd view it as definitely just a workaround and not a valid-from-first-principles approach. So even if that works I'd still argue that there's a bug in rules_js.

[alexeagle]

Discussion from Slack: rules_js deviates from pnpm behavior. If you have a dep "first-party": "workspace:*", pnpm will link node_modules/first-party to the source dir for first-party. Whereas rules_js links it to the virtual store

choose whether you want the symlink to point to the virtual store, like how the package would behave as a 3p dep (use npm_package) or to point to the source tree (use js_library)

@gregmagolan see this wiring work with rules_js with Bazel 6+ since we now have undeclared symlinks so we could define the bazel-bin/app/node_modules/component-lib symlink to be ../../component-lib and instead of an npm_package targets for the 1p dep, users would create a js_library target that would include what they want in the package as well as the :node_modules target to get the transitive deps

[Aghassi]

What's the thought process behind treating these things as different? Seems confusing for packages that are depended on by multiple targets to be considered a js_library instead of an npm_package given the ecosystem paradigm with workspaces:*

[Aghassi]

Also one other thought on this that our company will hit if everything ends up in dependencies it makes tools like Snyk assume things are getting shipped to prod. While this is a security issue and can be worked around, it is a callout from a developer experience prospective when it comes to accurately reflecting the state of the ecosystem in a repo

[gregmagolan]

A principled fix for this issue coming in #1646

[alexeagle]

@gregmagolan is this resolved with #1646 landed?

[gregmagolan]

@gregmagolan is this resolved with #1646 landed?

Yes and no. #1646 should allow users to resolve this if they switch to linking with js_library instead of npm_package. The linking with js_library matches how pnpm links 1p deps into node_modules so peer dep resolution within 1p deps should be the same with that method as pnpm. The fix, however, require JsInfo provider changes so it landed on the 2.x branch and will only be available with rules_js 2.0.

[alexeagle]

Now that 2.0 is shipping, marking this complete.