feat: add pnpm_version_from#2349
Conversation
|
alexeagle
force-pushed
the
pnpm_version_from
branch
from
adc3724 to
23eb35c
Compare
Ideally pnpm version should be kept in-sync with non-Bazel codepaths, and the easiest way to do this is the packageManager field in package.json
alexeagle
force-pushed
the
pnpm_version_from
branch
from
23eb35c to
8584284
Compare
| # Avoid inserting the default version from a non-root module | ||
| # (likely rules_js itself) if the root module already has a version. | ||
| if mod.is_root or len(registrations[attr.name]) == 0: | ||
| registrations[attr.name].append(v) |
There was a problem hiding this comment.
@alexeagle it seems to me that this code block essentially breaks all non-root module registrations. Note that:
- There is always a registration for
attr.name == "pnpm"(the one from rules_js itself) - It is not allowed to register a different name (see above).
Do you recall why you added this (it is not clear to me how it relates to the PR).
Sorry for the comment on the old PR, I'll file a bug report once I get a chance, but wanted to share the finding anyways.
There was a problem hiding this comment.
I think you are correct, and I have a fix WIP: #2883
The default `pnpm` repo was always taking priority and could not be overridden. See #2349 (comment) ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases - New test cases added
4 participants
Ideally pnpm version should be kept in-sync with non-Bazel codepaths, and the easiest way to do this is the packageManager field in package.json
See https://github.com/nodejs/corepack#when-authoring-packages