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

feat: default package_path to the directory of the package.json file in yarn_install and npm_install #3233

Merged
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
22 changes: 6 additions & 16 deletions docs/Built-ins.md
Original file line number Diff line number Diff line change
Expand Up @@ -658,17 +658,13 @@ Defaults to `[]`

A mapping of npm package names to bazel targets to linked into node_modules.

If `package_path` is also set, the bazel target will be linked to the node_modules at `package_path`
along with other 3rd party npm packages from this rule.

For example,

```
yarn_install(
name = "npm",
package_json = "//web:package.json",
yarn_lock = "//web:yarn.lock",
package_path = "web",
links = {
"@scope/target": "//some/scoped/target",
"target": "//some/target",
Expand Down Expand Up @@ -802,11 +798,10 @@ Defaults to `{}`

<h4 id="npm_install-package_path">package_path</h4>

(*String*): If set, link the 3rd party node_modules dependencies under the package path specified.
(*String*): The directory to link `node_modules` to in the execroot and in runfiles.

In most cases, this should be the directory of the package.json file so that the linker links the node_modules
in the same location they are found in the source tree. In a future release, this will default to the package.json
directory. This is planned for 4.0: https://github.com/bazelbuild/rules_nodejs/issues/2451
If unset, link `node_modules` to the directory of the `package.json` file specified in the
`package_json` attribute. Set to "/" to link to the root directory.

Defaults to `""`

Expand Down Expand Up @@ -1345,17 +1340,13 @@ Defaults to `[]`

A mapping of npm package names to bazel targets to linked into node_modules.

If `package_path` is also set, the bazel target will be linked to the node_modules at `package_path`
along with other 3rd party npm packages from this rule.

For example,

```
yarn_install(
name = "npm",
package_json = "//web:package.json",
yarn_lock = "//web:yarn.lock",
package_path = "web",
links = {
"@scope/target": "//some/scoped/target",
"target": "//some/target",
Expand Down Expand Up @@ -1473,11 +1464,10 @@ Defaults to `{}`

<h4 id="yarn_install-package_path">package_path</h4>

(*String*): If set, link the 3rd party node_modules dependencies under the package path specified.
(*String*): The directory to link `node_modules` to in the execroot and in runfiles.

In most cases, this should be the directory of the package.json file so that the linker links the node_modules
in the same location they are found in the source tree. In a future release, this will default to the package.json
directory. This is planned for 4.0: https://github.com/bazelbuild/rules_nodejs/issues/2451
If unset, link `node_modules` to the directory of the `package.json` file specified in the
`package_json` attribute. Set to "/" to link to the root directory.

Defaults to `""`

Expand Down
4 changes: 4 additions & 0 deletions e2e/packages/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ npm_install(
exports_directories_only = False,
package_json = "//:npm1/package.json",
package_lock_json = "//:npm1/package-lock.json",
package_path = "/",
)

npm_install(
Expand All @@ -35,6 +36,7 @@ npm_install(
exports_directories_only = False,
package_json = "//:npm2/package.json",
package_lock_json = "//:npm2/package-lock.json",
package_path = "/",
)

yarn_install(
Expand All @@ -43,6 +45,7 @@ yarn_install(
data = ["//:postinstall.js"],
exports_directories_only = False,
package_json = "//:yarn1/package.json",
package_path = "/",
yarn_lock = "//:yarn1/yarn.lock",
)

Expand All @@ -52,5 +55,6 @@ yarn_install(
data = ["//:postinstall.js"],
exports_directories_only = False,
package_json = "//:yarn2/package.json",
package_path = "/",
yarn_lock = "//:yarn2/yarn.lock",
)
23 changes: 14 additions & 9 deletions internal/npm_install/npm_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ See discussion in the README.
load("@rules_nodejs//nodejs/private:os_name.bzl", "is_windows_os", "os_name")
load("@rules_nodejs//nodejs/private:node_labels.bzl", "get_node_label", "get_npm_label")
load("//:version.bzl", "VERSION")
load("//third_party/github.com/bazelbuild/bazel-skylib:lib/paths.bzl", "paths")

COMMON_ATTRIBUTES = dict(dict(), **{
"data": attr.label_list(
Expand Down Expand Up @@ -134,17 +135,13 @@ as well as the fine grained targets such as `@wksp//foo`.

A mapping of npm package names to bazel targets to linked into node_modules.

If `package_path` is also set, the bazel target will be linked to the node_modules at `package_path`
along with other 3rd party npm packages from this rule.

For example,

```
yarn_install(
name = "npm",
package_json = "//web:package.json",
yarn_lock = "//web:yarn.lock",
package_path = "web",
links = {
"@scope/target": "//some/scoped/target",
"target": "//some/target",
Expand Down Expand Up @@ -210,11 +207,10 @@ fine grained npm dependencies.
),
"package_path": attr.string(
default = "",
doc = """If set, link the 3rd party node_modules dependencies under the package path specified.
doc = """The directory to link `node_modules` to in the execroot and in runfiles.

In most cases, this should be the directory of the package.json file so that the linker links the node_modules
in the same location they are found in the source tree. In a future release, this will default to the package.json
directory. This is planned for 4.0: https://github.com/bazelbuild/rules_nodejs/issues/2451""",
If unset, link `node_modules` to the directory of the `package.json` file specified in the
`package_json` attribute. Set to "/" to link to the root directory.""",
),
"patch_args": attr.string_list(
default = ["-p0"],
Expand Down Expand Up @@ -451,6 +447,15 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file, generate_loc
if not v.startswith("@"):
fail("link target must be label of form '@wksp//path/to:target', '@//path/to:target' or '//path/to:target'")
validated_links[k] = v

package_path = repository_ctx.attr.package_path
if not package_path:
# By default the package_path is the directory of the package.json file
package_path = paths.dirname(paths.join(repository_ctx.attr.package_json.package, repository_ctx.attr.package_json.name))
elif package_path == "/":
# User specified root path
package_path = ""

generate_config_json = json.encode(
struct(
exports_directories_only = repository_ctx.attr.exports_directories_only,
Expand All @@ -459,7 +464,7 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file, generate_loc
links = validated_links,
package_json = str(repository_ctx.path(repository_ctx.attr.package_json)),
package_lock = str(repository_ctx.path(lock_file)),
package_path = repository_ctx.attr.package_path,
package_path = package_path,
rule_type = rule_type,
strict_visibility = repository_ctx.attr.strict_visibility,
workspace = repository_ctx.attr.name,
Expand Down
22 changes: 7 additions & 15 deletions npm_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ js_library(
"@test_multi_linker/lib-d2": "@//internal/linker/test/multi_linker/lib_d",
},
package_json = "//internal/linker/test/multi_linker:package.json",
package_path = "internal/linker/test/multi_linker",
yarn_lock = "//internal/linker/test/multi_linker:yarn.lock",
)

Expand All @@ -155,28 +154,24 @@ js_library(
"@test_multi_linker/lib-d2": "@//internal/linker/test/multi_linker/lib_d",
},
package_json = "//internal/linker/test/multi_linker/test_a:package.json",
package_path = "internal/linker/test/multi_linker/test_a",
yarn_lock = "//internal/linker/test/multi_linker/test_a:yarn.lock",
)

yarn_install(
name = "internal_test_multi_linker_test_b_deps",
package_json = "//internal/linker/test/multi_linker/test_b:package.json",
package_path = "internal/linker/test/multi_linker/test_b",
yarn_lock = "//internal/linker/test/multi_linker/test_b:yarn.lock",
)

yarn_install(
name = "internal_test_multi_linker_test_c_deps",
package_json = "//internal/linker/test/multi_linker/test_c:package.json",
package_path = "internal/linker/test/multi_linker/test_c",
yarn_lock = "//internal/linker/test/multi_linker/test_c:yarn.lock",
)

yarn_install(
name = "internal_test_multi_linker_test_d_deps",
package_json = "//internal/linker/test/multi_linker/test_d:package.json",
package_path = "internal/linker/test/multi_linker/test_d",
yarn_lock = "//internal/linker/test/multi_linker/test_d:yarn.lock",
)

Expand All @@ -185,7 +180,6 @@ js_library(
# transitive deps for this first party lib should not include dev dependencies
args = ["--production"],
package_json = "//internal/linker/test/multi_linker/lib_b:package.json",
package_path = "internal/linker/test/multi_linker/lib_b",
yarn_lock = "//internal/linker/test/multi_linker/lib_b:yarn.lock",
)

Expand All @@ -194,7 +188,6 @@ js_library(
# transitive deps for this first party lib should not include dev dependencies
args = ["--production"],
package_json = "//internal/linker/test/multi_linker/lib_c:lib/package.json",
package_path = "internal/linker/test/multi_linker/lib_c/lib",
yarn_lock = "//internal/linker/test/multi_linker/lib_c:lib/yarn.lock",
)

Expand Down Expand Up @@ -230,7 +223,6 @@ js_library(
"@test_multi_linker/lib-d2": "@build_bazel_rules_nodejs//internal/linker/test/multi_linker/lib_d",
},
package_json = "//internal/linker/test/multi_linker/sub:package.json",
package_path = "internal/linker/test/multi_linker/sub",
yarn_lock = "//internal/linker/test/multi_linker/sub:yarn.lock",
)

Expand All @@ -239,7 +231,6 @@ js_library(
# transitive deps for this first party lib should not include dev dependencies
args = ["--production"],
package_json = "//internal/linker/test/multi_linker/onep_a:package.json",
package_path = "internal/linker/test/multi_linker/onep_a",
yarn_lock = "//internal/linker/test/multi_linker/onep_a:yarn.lock",
)

Expand All @@ -264,6 +255,7 @@ js_library(
],
package_json = "//:tools/fine_grained_deps_yarn/package.json",
yarn_lock = "//:tools/fine_grained_deps_yarn/yarn.lock",
package_path = "/",
exports_directories_only = False,
)

Expand All @@ -289,6 +281,7 @@ js_library(
npm_command = "install",
package_json = "//:tools/fine_grained_deps_npm/package.json",
package_lock_json = "//:tools/fine_grained_deps_npm/package-lock.json",
package_path = "/",
exports_directories_only = False,
)

Expand All @@ -301,6 +294,7 @@ js_library(
"SOME_USER_ENV": "yarn is great!",
},
package_json = "//:tools/fine_grained_deps_yarn_directory_artifacts/package.json",
package_path = "/",
yarn_lock = "//:tools/fine_grained_deps_yarn_directory_artifacts/yarn.lock",
)

Expand All @@ -314,12 +308,14 @@ js_library(
},
npm_command = "install",
package_json = "//:tools/fine_grained_deps_npm_directory_artifacts/package.json",
package_path = "/",
package_lock_json = "//:tools/fine_grained_deps_npm_directory_artifacts/package-lock.json",
)

yarn_install(
name = "fine_grained_no_bin",
package_json = "//:tools/fine_grained_no_bin/package.json",
package_path = "/",
yarn_lock = "//:tools/fine_grained_no_bin/yarn.lock",
)

Expand Down Expand Up @@ -368,6 +364,7 @@ filegroup(
],
)""",
package_json = "//:tools/fine_grained_goldens/package.json",
package_path = "/",
yarn_lock = "//:tools/fine_grained_goldens/yarn.lock",
exports_directories_only = False,
)
Expand Down Expand Up @@ -407,13 +404,13 @@ filegroup(
],
)""",
package_json = "//:tools/fine_grained_goldens/package.json",
package_path = "/",
yarn_lock = "//:tools/fine_grained_goldens/yarn.lock",
)

yarn_install(
name = "internal_npm_install_test_patches_yarn",
package_json = "//internal/npm_install/test/patches_yarn:package.json",
package_path = "internal/npm_install/test/patches_yarn",
patch_args = ["-p0"],
patch_tool = "patch",
post_install_patches = ["//internal/npm_install/test/patches_yarn:semver+1.0.0.patch"],
Expand All @@ -439,7 +436,6 @@ filegroup(
name = "internal_npm_install_test_patches_npm",
package_json = "//internal/npm_install/test/patches_npm:package.json",
package_lock_json = "//internal/npm_install/test/patches_npm:package-lock.json",
package_path = "internal/npm_install/test/patches_npm",
patch_args = ["-p0"],
patch_tool = "patch",
post_install_patches = ["//internal/npm_install/test/patches_npm:semver+1.0.0.patch"],
Expand All @@ -463,7 +459,6 @@ filegroup(
yarn_install(
name = "internal_npm_install_test_patches_yarn_symlinked",
package_json = "//internal/npm_install/test/patches_yarn_symlinked:package.json",
package_path = "internal/npm_install/test/patches_yarn_symlinked",
patch_args = ["-p0"],
patch_tool = "patch",
post_install_patches = ["//internal/npm_install/test/patches_yarn_symlinked:semver+1.0.0.patch"],
Expand All @@ -476,7 +471,6 @@ filegroup(
name = "internal_npm_install_test_patches_npm_symlinked",
package_json = "//internal/npm_install/test/patches_npm_symlinked:package.json",
package_lock_json = "//internal/npm_install/test/patches_npm_symlinked:package-lock.json",
package_path = "internal/npm_install/test/patches_npm_symlinked",
patch_args = ["-p0"],
patch_tool = "patch",
post_install_patches = ["//internal/npm_install/test/patches_npm_symlinked:semver+1.0.0.patch"],
Expand Down Expand Up @@ -529,7 +523,6 @@ filegroup(
],
)""",
package_json = "//:tools/fine_grained_goldens/package.json",
package_path = "tools/fine_grained_goldens",
yarn_lock = "//:tools/fine_grained_goldens/yarn.lock",
)

Expand All @@ -554,6 +547,5 @@ filegroup(
yarn_install(
name = "rollup_test_multi_linker_deps",
package_json = "//packages/rollup/test/multi_linker:package.json",
package_path = "packages/rollup/test/multi_linker",
yarn_lock = "//packages/rollup/test/multi_linker:yarn.lock",
)