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

Split up npm fine grained deps BUILD file into multiple BUILD files #337

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
3 changes: 0 additions & 3 deletions .circleci/bazel.rc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
# Don't be spammy in the logs
build --noshow_progress

# Don't run manual tests
test --test_tag_filters=-manual

# Print all the options that apply to the build.
# This helps us diagnose which options override others
# (e.g. /etc/bazel.bazelrc vs. tools/bazel.rc)
Expand Down
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ jobs:

- run: bazel run @nodejs//:yarn
- run: bazel build ...
- run: bazel test ... --define=some_env=some_value
- run: bazel test ...
# TODO(alexeagle): move this into the example proper
- run: bazel run examples/rollup -- --help

Expand All @@ -90,6 +90,7 @@ jobs:
- run: bazel run //internal/node/test:has_deps_legacy
- run: bazel run //internal/node/test:has_deps
- run: bazel run //internal/node/test:has_deps_hybrid
- run: bazel run @fine_grained_deps_yarn//typescript/bin:tsc

# We should also be able to test targets in a different workspace
- run: bazel test @program_example//...
Expand Down
66 changes: 57 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
nodejs_binary(
name = "bar",
data = [
"@npm//:foo",
"@npm//:baz",
"@npm//foo",
"@npm//baz",
]
...
)
Expand All @@ -220,14 +220,57 @@ jasmine_node_test(
name = "test",
...
deps = [
"@npm//:jasmine",
"@npm//:foo",
"@npm//:baz",
"@npm//jasmine",
"@npm//foo",
"@npm//baz",
...
],
)
```

#### Fine-grained npm package nodejs_binary targets

If an npm package lists one of more `bin` entry points in its `package.json`,
`nodejs_binary` targets will be generated for these.

For example, the `protractor` package has two bin entries in its `package.json`:

```json
"bin": {
"protractor": "bin/protractor",
"webdriver-manager": "bin/webdriver-manager"
},
```

These will result in two generated `nodejs_binary` targets in the `@npm//protractor/bin`
package (if your npm deps workspace is `@npm`):

```python
nodejs_binary(
name = "protractor",
entry_point = "protractor/bin/protractor",
data = ["//protractor"],
)

nodejs_binary(
name = "webdriver-manager",
entry_point = "protractor/bin/webdriver-manager",
data = ["//protractor"],
)
```

These targets can be used as executables for actions in custom rules or can
be run by Bazel directly. For example, you can run protractor with the
following:

```sh
$ bazel run @npm//protractor/bin:protractor
```

Note: These targets are in the `protractor/bin` package so they don't
conflict with the targets to use in deps[]. For example, `@npm//protractor:protractor`
is target to use in deps[] while `@npm//protractor/bin:protractor` is the binary target.

#### Coarse node_modules dependencies

Using fine grained npm dependencies is recommended to minimize
Expand All @@ -236,15 +279,14 @@ there are also filegroups defined by `yarn_install` and `npm_install`
that include all packages under `node_modules` and which can be used
with the `node_modules` attribute of nodejs rules.

* `@npm//:node_modules` is includes **all** files under `node_modules`
* `@npm//:node_modules_lite` is includes only `.js`, `.d.ts`, `.json` and `./bin/*` under `node_modules` (this reduces the number of input files)
* `@npm//:node_modules` includes all packages under `node_modules` as well as the `.bin` folder

```python
load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")

nodejs_binary(
name = "bar",
node_modules = "@npm//:node_modules_lite",
node_modules = "@npm//:node_modules",
)
```

Expand All @@ -260,8 +302,14 @@ filegroup(
name = "node_modules",
srcs = glob(
include = ["node_modules/**/*"],
# Files with spaces in the name are not legal Bazel labels
exclude = [
# Files under test & docs may contain file names that
# are not legal Bazel labels (e.g.,
# node_modules/ecstatic/test/public/中文/檔案.html)
"node_modules/test/**",
"node_modules/docs/**",
# Files with spaces are not allowed in Bazel runfiles
# See https://github.com/bazelbuild/bazel/issues/4327
"node_modules/**/* */**",
"node_modules/**/* *",
],
Expand Down
4 changes: 2 additions & 2 deletions examples/bazel_managed_deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jasmine_node_test(
name = "fine_grained_test",
srcs = glob(["*.spec.js"]),
deps = [
"@npm//:jasmine",
"@npm//:typescript",
"@npm//jasmine",
"@npm//typescript",
],
)
2 changes: 1 addition & 1 deletion examples/bazel_managed_deps/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Instead, we can declare individual npm packages as dependencies, e.g.:
```
nodejs_binary(
name = "fast",
data = ["@npm//:jasmine"]
data = ["@npm//jasmine"]
)
```

Expand Down
2 changes: 1 addition & 1 deletion examples/bazel_managed_deps/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ node_repositories()
# This runs yarn install, then our generate_build_file.js to create BUILD files
# inside the resulting node_modules directory.
# The name "npm" here means the resulting modules are referenced like
# @npm//:jasmine
# @npm//jasmine
yarn_install(
name = "npm",
package_json = "//:package.json",
Expand Down
9 changes: 5 additions & 4 deletions examples/define_var/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ jasmine_node_test(
name = "define_var",
srcs = glob(["*.spec.js"]),
# Just here as a smoke test for this attribute.
# Test must be run with --define=some_env=some_value.
# Use `bazel test ... --define=some_env=some_value` or
# `bazel test //examples/rollup:test --define=some_env=some_value`.
# This test must be run with --define=some_env=some_value.
# Use `bazel test //examples/define_var --define=some_env=some_value`.
# Note that use of --define causes the entire build to be non-incremental
# since --define can affect the output of any action.
configuration_env_vars = ["some_env"],
data = ["@fine_grained_deps_yarn//:jasmine"],
data = ["@fine_grained_deps_yarn//jasmine"],
# Don't include this test in //... pattern since it requires special
# --define option
tags = ["manual"],
Expand Down
6 changes: 3 additions & 3 deletions internal/e2e/fine_grained_deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test")
"fine.spec.js",
],
deps = [
"@fine_grained_deps_%s//:jasmine" % pkgmgr,
"@fine_grained_deps_%s//:typescript" % pkgmgr,
"@fine_grained_deps_%s//jasmine" % pkgmgr,
"@fine_grained_deps_%s//typescript" % pkgmgr,
# Note, test-b depends on [email protected] which should get
# hoisted to node_modules/test-b/node_modules/test-a
"@fine_grained_deps_%s//:@gregmagolan/test-b" % pkgmgr,
"@fine_grained_deps_%s//@gregmagolan/test-b" % pkgmgr,
],
) for pkgmgr in [
"yarn",
Expand Down
12 changes: 6 additions & 6 deletions internal/e2e/rollup_fine_grained_deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ rollup_bundle(
srcs = ["has-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
deps = [
"@fine_grained_deps_yarn//:@gregmagolan/test-b",
"@fine_grained_deps_yarn//@gregmagolan/test-b",
],
)

Expand All @@ -25,7 +25,7 @@ rollup_bundle(
name = "bundle_legacy",
srcs = ["has-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
node_modules = "@fine_grained_deps_yarn//:node_modules",
)

# You can have a rollup_bundle with both a node_modules attribute
Expand All @@ -34,9 +34,9 @@ rollup_bundle(
name = "bundle_hybrid",
srcs = ["has-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
node_modules = "@fine_grained_deps_yarn//:node_modules",
deps = [
"@fine_grained_deps_yarn//:@gregmagolan/test-b",
"@fine_grained_deps_yarn//@gregmagolan/test-b",
],
)

Expand All @@ -60,7 +60,7 @@ jasmine_node_test(
":bundle_no_deps.js",
":bundle_no_deps.min.js",
":bundle_no_deps.min_debug.js",
"@fine_grained_deps_yarn//:@gregmagolan/test-a",
"@fine_grained_deps_yarn//:jasmine",
"@fine_grained_deps_yarn//@gregmagolan/test-a",
"@fine_grained_deps_yarn//jasmine",
],
)
4 changes: 2 additions & 2 deletions internal/jasmine_node_test/jasmine_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ function main(args) {
.filter(l => l.length > 0)
// Filter here so that only files ending in `spec.js` and `test.js`
// are added to jasmine as spec files. This is important as other
// deps such as "@npm//:typescript" if executed may cause the test to
// fail or have unexpected side-effects. "@npm//:typescript" would
// deps such as "@npm//typescript" if executed may cause the test to
// fail or have unexpected side-effects. "@npm//typescript" would
// try to execute tsc, print its help, and process.exit(1)
.filter(f => /[^a-zA-Z0-9](spec|test)\.js$/i.test(f))
// Filter out files from node_modules that match test.js or spec.js
Expand Down
10 changes: 5 additions & 5 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ _NODEJS_EXECUTABLE_ATTRS = {
name = "my_binary",
...
data = [
"@npm//:foo",
"@npm//:bar",
"@npm//foo",
"@npm//bar",
...
],
)
Expand Down Expand Up @@ -228,9 +228,9 @@ _NODEJS_EXECUTABLE_ATTRS = {
name = "my_test",
...
deps = [
"@npm//:jasmine",
"@npm//:foo",
"@npm//:bar",
"@npm//jasmine",
"@npm//foo",
"@npm//bar",
...
],
)
Expand Down
8 changes: 4 additions & 4 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ nodejs_binary(
name = "has_deps_legacy",
data = ["has-deps.js"],
entry_point = "build_bazel_rules_nodejs/internal/node/test/has-deps",
node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
node_modules = "@fine_grained_deps_yarn//:node_modules",
)

# You can have a nodejs_binary with no node_modules attribute
Expand All @@ -23,7 +23,7 @@ nodejs_binary(
name = "has_deps",
data = [
"has-deps.js",
"@fine_grained_deps_yarn//:typescript",
"@fine_grained_deps_yarn//typescript",
],
entry_point = "build_bazel_rules_nodejs/internal/node/test/has-deps",
)
Expand All @@ -34,8 +34,8 @@ nodejs_binary(
name = "has_deps_hybrid",
data = [
"has-deps.js",
"@fine_grained_deps_yarn//:typescript",
"@fine_grained_deps_yarn//typescript",
],
entry_point = "build_bazel_rules_nodejs/internal/node/test/has-deps",
node_modules = "@fine_grained_deps_yarn//:node_modules_lite",
node_modules = "@fine_grained_deps_yarn//:node_modules",
)
Loading