Skip to content

Commit

Permalink
feat: default package_path to the directory of the package.json file …
Browse files Browse the repository at this point in the history
…in yarn_install and npm_install
  • Loading branch information
gregmagolan committed Jan 13, 2022
1 parent 77ab690 commit 273405b
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 40 deletions.
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 @@ -25,6 +25,7 @@ load("@rules_nodejs//nodejs/private:os_name.bzl", "is_windows_os", "os_name")
load("//:version.bzl", "VERSION")
load("//internal/common:check_bazel_version.bzl", "check_bazel_version")
load("//internal/node:node_labels.bzl", "get_node_label", "get_npm_label", "get_yarn_label")
load("//third_party/github.com/bazelbuild/bazel-skylib:lib/paths.bzl", "paths")

COMMON_ATTRIBUTES = dict(dict(), **{
"data": attr.label_list(
Expand Down Expand Up @@ -135,17 +136,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 @@ -211,11 +208,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 @@ -452,6 +448,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 @@ -460,7 +465,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
24 changes: 9 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,21 +523,22 @@ filegroup(
],
)""",
package_json = "//:tools/fine_grained_goldens/package.json",
package_path = "tools/fine_grained_goldens",
yarn_lock = "//:tools/fine_grained_goldens/yarn.lock",
)

npm_install(
name = "npm_node_patches",
package_json = "//packages/node-patches:package.json",
package_lock_json = "//packages/node-patches:package-lock.json",
package_path = "/",
# TODO: fix tests when this flag is flipped
exports_directories_only = False,
)

yarn_install(
name = "cypress_deps",
package_json = "//packages/cypress/test:package.json",
package_path = "/",
yarn_lock = "//packages/cypress/test:yarn.lock",
# TODO: get cypress rule working with symlink_node_modules = False
symlink_node_modules = True,
Expand All @@ -554,6 +549,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",
)

0 comments on commit 273405b

Please sign in to comment.