Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move npm_registry_url to utils #1758

Merged
merged 1 commit into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions e2e/pnpm_lockfiles/.bazelignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
node_modules/
cases/node_modules
projects/a/node_modules
projects/b/node_modules
projects/c/node_modules
Expand Down
23 changes: 23 additions & 0 deletions e2e/pnpm_lockfiles/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@ local_path_override(
path = "../..",
)

# The same primary lockfile across all versions
PNPM_LOCK_VERSIONS = [
"v54",
"v60",
"v61",
"v90",
]

# Lockfiles with unique test cases
PNPM_LOCK_TEST_CASES = [
"tarball-no-url-v54.yaml",
]

bazel_dep(name = "aspect_bazel_lib", version = "2.7.6")

bazel_dep(name = "bazel_skylib", version = "1.5.0", dev_dependency = True)
Expand Down Expand Up @@ -62,3 +68,20 @@ npm = use_extension(
)
for version in PNPM_LOCK_VERSIONS
]

[
npm.npm_translate_lock(
name = lockfile.replace(".yaml", ""),
pnpm_lock = "//cases:%s" % lockfile,
verify_node_modules_ignored = "//:.bazelignore",
)
for lockfile in PNPM_LOCK_TEST_CASES
]

[
use_repo(
npm,
lockfile.replace(".yaml", ""),
)
for lockfile in PNPM_LOCK_TEST_CASES
]
6 changes: 6 additions & 0 deletions e2e/pnpm_lockfiles/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,9 @@ TODO:
No :node_modules/\* targets are generated for aliases to npm packages.

Note: _sometimes_ fails to install with pnpm9

## pnpm lockfile edge cases

Unique test cases hard to cover with normal pnpm workspaces + package.json. Each
test case is a pnpm-lock.yaml with a unique filename, see cases/BUILD for how the test
cases run on each of those lockfiles.
19 changes: 19 additions & 0 deletions e2e/pnpm_lockfiles/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@ local_repository(
path = "../..",
)

# The same primary lockfile across all versions
PNPM_LOCK_VERSIONS = [
"v54",
"v60",
"v61",
"v90",
]

# Lockfiles with unique test cases
PNPM_LOCK_TEST_CASES = [
"tarball-no-url-v54.yaml",
]

load("@aspect_rules_js//js:repositories.bzl", "rules_js_dependencies")

rules_js_dependencies()
Expand Down Expand Up @@ -45,3 +51,16 @@ npm_repositories_v60()
npm_repositories_v61()

npm_repositories_v90()

[
npm_translate_lock(
name = lockfile.replace(".yaml", ""),
pnpm_lock = "//cases:%s" % lockfile,
verify_node_modules_ignored = "//:.bazelignore",
)
for lockfile in PNPM_LOCK_TEST_CASES
]

load("@tarball-no-url-v54//:repositories.bzl", npm_repositories_tarball_no_url_v54 = "npm_repositories")

npm_repositories_tarball_no_url_v54()
14 changes: 14 additions & 0 deletions e2e/pnpm_lockfiles/cases/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@tarball-no-url-v54//:defs.bzl", tarball_no_url_link_all = "npm_link_all_packages")

tarball_no_url_link_all(name = "tarball-no-url-v54-modules")

build_test(
name = "tarball-no-url-v54",
targets = [
":tarball-no-url-v54-modules",
":tarball-no-url-v54-modules/@aspect-build/a",
],
)

exports_files(glob(["*.yaml"]))
1 change: 1 addition & 0 deletions e2e/pnpm_lockfiles/cases/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
2 changes: 2 additions & 0 deletions e2e/pnpm_lockfiles/cases/pnpm-workspace.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
packages:
- '.'
19 changes: 19 additions & 0 deletions e2e/pnpm_lockfiles/cases/tarball-no-url-v54.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
lockfileVersion: 5.4

importers:
.:
specifiers:
'@aspect-build/a': 1.0.0
dependencies:
'@aspect-build/a': 1.0.0

packages:
# This was found in https://github.com/aspect-build/rules_js/blob/795138561e878c09f0b62a25572358d64960d732/e2e/npm_translate_lock_auth/pnpm-lock.yaml
/@aspect-build/a/1.0.0:
resolution:
{
integrity: sha512-MYeL/yqAPYJXOnnSEOAdtQSu/8tiifFtKN6Jg/rgpKRqxKL8NVsQXrX9H2dlJ4mS23pu7VS0+i9mZNiRoCUYwg==,
tarball: '@aspect-test/a/-/a-1.0.0.tgz',
gregmagolan marked this conversation as resolved.
Show resolved Hide resolved
}
hasBin: true
dev: false
12 changes: 6 additions & 6 deletions npm/private/test/utils_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -92,32 +92,32 @@ def test_npm_registry_url(ctx):
asserts.equals(
env,
"https://default",
utils_test.npm_registry_url("a", {}, "https://default"),
utils.npm_registry_url("a", {}, "https://default"),
)
asserts.equals(
env,
"http://default",
utils_test.npm_registry_url("a", {}, "http://default"),
utils.npm_registry_url("a", {}, "http://default"),
)
asserts.equals(
env,
"//default",
utils_test.npm_registry_url("a", {}, "//default"),
utils.npm_registry_url("a", {}, "//default"),
)
asserts.equals(
env,
"https://default",
utils_test.npm_registry_url("@a/b", {}, "https://default"),
utils.npm_registry_url("@a/b", {}, "https://default"),
)
asserts.equals(
env,
"https://default",
utils_test.npm_registry_url("@a/b", {"@ab": "not me"}, "https://default"),
utils.npm_registry_url("@a/b", {"@ab": "not me"}, "https://default"),
)
asserts.equals(
env,
"https://scoped-registry",
utils_test.npm_registry_url("@a/b", {"@a": "https://scoped-registry"}, "https://default"),
utils.npm_registry_url("@a/b", {"@a": "https://scoped-registry"}, "https://default"),
)
return unittest.end(env)

Expand Down
2 changes: 1 addition & 1 deletion npm/private/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ utils = struct(
links_repo_suffix = "__links",
# Output group name for the package directory of a linked npm package
package_directory_output_group = "package_directory",
npm_registry_url = _npm_registry_url,
npm_registry_download_url = _npm_registry_download_url,
is_git_repository_url = _is_git_repository_url,
to_registry_url = _to_registry_url,
Expand All @@ -756,7 +757,6 @@ utils = struct(

# Exported only to be tested
utils_test = struct(
npm_registry_url = _npm_registry_url,
parse_package_name = _parse_package_name,
strip_v5_peer_dep_or_patched_version = _strip_v5_peer_dep_or_patched_version,
)
Loading