rules_js
rules_js copied to clipboard
aspect-build
[Bug]: Dependency aliases of transitive dependencies are not working
What happened?
Consider you are using a project that has a direct dependency on A. Where A then has a dependency on B, but the package is aliased to e.g. alias. e.g.
registry.npmjs.org/@isaacs/cliui/8.0.2:
name: '@isaacs/cliui'
version: 8.0.2
dependencies:
string-width-cjs: registry.npmjs.org/string-width/4.2.3
rules_js will properly configure the store target for cliui to look for string-width-cjs as a store dependency. But the store target with the aliased name does actually not exist.
This causes errors like:
ERROR: /usr/local/google/home/pgschwendtner/projects/material.angular.io/BUILD.bazel:9:22: in deps attribute
of npm_package_store_internal rule
//:.aspect_rules_js/node_modules/@[email protected]+@isaacs+cliui+8.0.2/pkg: \
target '//:.aspect_rules_js/node_modules/[email protected]+string-width+4.2.3/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
Version
Linux amd64. Rules_JS rules_js-1.20.1. Bazel 5.1.0. A team member also tried with the latest version.
How to reproduce
No response
Any other information?
Looking into it without diving too much into implementation, the issue seems to be that pnpm_lock_file.packages() is used to create the store entries. But this never holds the aliased package names. i.e. rules_js iterates through the packages: entry in the lock file and then creates store entries for those.
There will never be an entry with the alias though because the alias name will only ever appear in the dependencies section of an entry. The transitive closure detection already has access to this information.
Likely we could use that to compile a more precise list of packages that will then be used
to create the links.
Have a repro here, https://github.com/aspect-build/rules_js/pull/1063. Working on a fix.
Hmm. It is a tricky one and more than an hour of work. I think this one will have to wait until next week when either @jbedard or I have more time.
Notes from repro in #1063...
In /private/var/tmp/_bazel_greg/7aed427a991d86f99332ab79b9b11780/external/npm/repositories.bzl the generated npm_import for the problematic @isaacs/[email protected] package looks like this,
npm_import(
name = "npm__at_isaacs_cliui__registry.npmjs.org_at_isaacs_cliui_8.0.2",
root_package = "",
link_workspace = "",
link_packages = {
"npm/private/test": ["@isaacs/cliui"],
},
package = "@isaacs/cliui",
version = "registry.npmjs.org/@isaacs/[email protected]",
url = "https://registry.yarnpkg.com/@isaacs/cliui/-/cliui-8.0.2.tgz",
npm_translate_lock_repo = "npm",
dev = True,
generate_bzl_library_targets = True,
integrity = "sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==",
deps = {
"string-width": "registry.npmjs.org/[email protected]",
"string-width-cjs": "registry.npmjs.org/[email protected]",
"strip-ansi": "registry.npmjs.org/[email protected]",
"strip-ansi-cjs": "registry.npmjs.org/[email protected]",
"wrap-ansi": "registry.npmjs.org/[email protected]",
"wrap-ansi-cjs": "registry.npmjs.org/[email protected]",
},
transitive_closure = {
"@isaacs/cliui": ["registry.npmjs.org/@isaacs/[email protected]"],
"string-width": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"string-width-cjs": ["registry.npmjs.org/[email protected]"],
"strip-ansi": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"strip-ansi-cjs": ["registry.npmjs.org/[email protected]"],
"wrap-ansi": ["registry.npmjs.org/[email protected]"],
"wrap-ansi-cjs": ["registry.npmjs.org/[email protected]"],
"ansi-styles": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"ansi-regex": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"emoji-regex": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"is-fullwidth-code-point": ["registry.npmjs.org/[email protected]"],
"color-convert": ["registry.npmjs.org/[email protected]"],
"color-name": ["registry.npmjs.org/[email protected]"],
"eastasianwidth": ["registry.npmjs.org/[email protected]"],
},
)
The transitive closure is calculated wrong from the looks of it, it should likely be:
transitive_closure = {
"@isaacs/cliui": ["registry.npmjs.org/@isaacs/[email protected]"],
"string-width": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"strip-ansi": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"wrap-ansi": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"ansi-styles": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"ansi-regex": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"emoji-regex": ["registry.npmjs.org/[email protected]", "registry.npmjs.org/[email protected]"],
"is-fullwidth-code-point": ["registry.npmjs.org/[email protected]"],
"color-convert": ["registry.npmjs.org/[email protected]"],
"color-name": ["registry.npmjs.org/[email protected]"],
"eastasianwidth": ["registry.npmjs.org/[email protected]"],
},
Although even then I think there will also be fixed needed in npm_import itself that generates links the /private/var/tmp/_bazel_greg/7aed427a991d86f99332ab79b9b11780/external/npm__at_isaacs_cliui__registry.npmjs.org_at_isaacs_cliui_8.0.2__links/defs.bzl for the package. In particular, the deps, lc_deps & ref_deps there which currently look like
deps = {
":.aspect_rules_js/{}/@[email protected]+@[email protected]/pkg".format(link_root_name): "@isaacs/cliui",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "is-fullwidth-code-point",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-convert",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-name",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "eastasianwidth",
}
lc_deps = {
":.aspect_rules_js/{}/@[email protected]+@[email protected]/pkg_pre_lc_lite".format(link_root_name): "@isaacs/cliui",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "is-fullwidth-code-point",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-convert",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-name",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "eastasianwidth",
}
ref_deps = {
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "string-width-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "strip-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "wrap-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "wrap-ansi-cjs",
}
The other key observation is that this issue ONLY happens when the package name in the pnpm lock file starts with registry.npmjs.org/ such as
registry.npmjs.org/@isaacs/[email protected]:
resolution: {integrity: sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==, registry: https://registry.yarnpkg.com/, tarball: https://registry.npmjs.org/@isaacs/cliui/-/cliui-8.0.2.tgz}
name: '@isaacs/cliui'
version: 8.0.2
engines: {node: '>=12'}
dependencies:
string-width: registry.npmjs.org/[email protected]
string-width-cjs: registry.npmjs.org/[email protected]
strip-ansi: registry.npmjs.org/[email protected]
strip-ansi-cjs: registry.npmjs.org/[email protected]
wrap-ansi: registry.npmjs.org/[email protected]
wrap-ansi-cjs: registry.npmjs.org/[email protected]
dev: true
which only happens when the .npmrc registry setting is set for the package in the repro. In the repro it is set to registry=https://registry.yarnpkg.com.
When registry is not set for the package in npmrc then the pnpm-lock.yaml file package entry looks like this:
/@isaacs/[email protected]:
resolution: {integrity: sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==}
engines: {node: '>=12'}
dependencies:
string-width: 5.1.2
string-width-cjs: /[email protected]
strip-ansi: 7.0.1
strip-ansi-cjs: /[email protected]
wrap-ansi: 8.1.0
wrap-ansi-cjs: /[email protected]
dev: true
with the aliases clearly distinguishable and rules_js correctly handles this default case.
The generated npm_import looks like this in this case,
npm_import(
name = "npm__at_isaacs_cliui__8.0.2",
root_package = "",
link_workspace = "",
link_packages = {
"npm/private/test": ["@isaacs/cliui"],
},
package = "@isaacs/cliui",
version = "8.0.2",
url = "https://registry.npmjs.org/@isaacs/cliui/-/cliui-8.0.2.tgz",
npm_translate_lock_repo = "npm",
dev = True,
generate_bzl_library_targets = True,
integrity = "sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==",
deps = {
"string-width": "5.1.2",
"string-width-cjs": "/string-width/4.2.3",
"strip-ansi": "7.0.1",
"strip-ansi-cjs": "/strip-ansi/6.0.1",
"wrap-ansi": "8.1.0",
"wrap-ansi-cjs": "/wrap-ansi/7.0.0",
},
transitive_closure = {
"@isaacs/cliui": ["8.0.2"],
"string-width": ["4.2.3", "5.1.2"],
"strip-ansi": ["6.0.1", "7.0.1"],
"wrap-ansi": ["7.0.0", "8.1.0"],
"ansi-styles": ["6.2.1", "4.3.0"],
"color-convert": ["2.0.1"],
"color-name": ["1.1.4"],
"ansi-regex": ["6.0.1", "5.0.1"],
"emoji-regex": ["9.2.2", "8.0.0"],
"is-fullwidth-code-point": ["3.0.0"],
"eastasianwidth": ["0.2.0"],
},
)
and the deps, lc_deps and ref_deps look like this,
deps = {
":.aspect_rules_js/{}/@[email protected]+@[email protected]/pkg".format(link_root_name): "@isaacs/cliui",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "is-fullwidth-code-point",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-convert",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-name",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "eastasianwidth",
}
lc_deps = {
":.aspect_rules_js/{}/@[email protected]+@[email protected]/pkg_pre_lc_lite".format(link_root_name): "@isaacs/cliui",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "string-width-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "strip-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "wrap-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-styles",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "ansi-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "emoji-regex",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "is-fullwidth-code-point",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-convert",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "color-name",
":.aspect_rules_js/{}/[email protected][email protected]/pkg".format(link_root_name): "eastasianwidth",
}
ref_deps = {
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "string-width",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "string-width-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "strip-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "strip-ansi-cjs",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "wrap-ansi",
":.aspect_rules_js/{}/[email protected][email protected]/ref".format(link_root_name): "wrap-ansi-cjs",
}
@devversion This is a potential work-around for the material repository: https://github.com/angular/material.angular.io/pull/1208/files
@gregmagolan Thanks for the investigation. So based on that, to me it sounds like the registry prefixes are currently just throwing off the logic, but in practice- with both variants (with registry prefix, or not), the aliases should be distinguishable, right? e.g.
with prefixes
registry.npmjs.org/@isaacs/[email protected]:
resolution: {integrity: sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==, registry: https://registry.yarnpkg.com/, tarball: https://registry.npmjs.org/@isaacs/cliui/-/cliui-8.0.2.tgz}
name: '@isaacs/cliui'
version: 8.0.2
engines: {node: '>=12'}
dependencies:
string-width: registry.npmjs.org/[email protected] <----- Same package name. No alias.
string-width-cjs: registry.npmjs.org/[email protected] <----- Different package name. Can be detected.
strip-ansi: registry.npmjs.org/[email protected]
strip-ansi-cjs: registry.npmjs.org/[email protected]
wrap-ansi: registry.npmjs.org/[email protected]
wrap-ansi-cjs: registry.npmjs.org/[email protected]
dev: true
without prefix
/@isaacs/[email protected]:
resolution: {integrity: sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==}
engines: {node: '>=12'}
dependencies:
string-width: 5.1.2 <----- No alias.
string-width-cjs: /[email protected] <----- Different package name. Can be detected.
strip-ansi: 7.0.1
strip-ansi-cjs: /[email protected]
wrap-ansi: 8.1.0
wrap-ansi-cjs: /[email protected]
dev: true
Essentially, yes. The logic breaks for the aliases somewhere along the way with the registry.npmjs.org/ prefixed versions. I started down the path of fixing it but it was more work than I had time for and with a viable work-around I'm going to put fixing the registry.npmjs.org/ prefix case on the backlog since we have other higher priority work at the moment.
FYI that team members often run into issues, even with the workaround, because pnpm import seems to behave different when running under yarn bazel. The registry prefixes are always added then- reverting the workarounds basically