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.

Sign up for GitHub

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

Add support for Label in nodejs_binary and rollup_bundle entry_point #777

Merged
merged 2 commits into from
Jun 6, 2019
Merged

Add support for Label in nodejs_binary and rollup_bundle entry_point #777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4eb617
feat: add support for Label in nodejs_binary#entry_point (#480)
manekinekko Apr 12, 2019
02e19b7
feat: fixes and get all tests passing for nodejs_binary & rollup_bund…
gregmagolan May 22, 2019
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
19 changes: 4 additions & 15 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,19 +320,8 @@ For example, the `protractor` package has two bin entries in its `package.json`:
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"],
)
```
* `@npm//protractor/bin:protractor`
* `@npm//protractor/bin:webdriver-manager`

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
Expand Down Expand Up @@ -452,7 +441,7 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")

nodejs_binary(
name = "rollup",
entry_point = "rollup/bin/rollup",
entry_point = "//:node_modules/rollup/bin/rollup",
)
```

Expand Down Expand Up @@ -480,7 +469,7 @@ nodejs_binary(
"@//:node_modules",
"main.js",
],
entry_point = "workspace_name/main.js",
entry_point = ":main.js",
args = ["--node_options=--expose-gc"],
)
```
Expand Down
2 changes: 1 addition & 1 deletion e2e/ts_library/googmodule/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ nodejs_binary(
"@npm//@bazel/typescript",
"@npm//tsickle",
],
entry_point = "@bazel/typescript/internal/tsc_wrapped/tsc_wrapped.js",
entry_point = "@npm//node_modules/@bazel/typescript:internal/tsc_wrapped/tsc_wrapped.js",
install_source_map_support = False,
)

Expand Down
2 changes: 1 addition & 1 deletion e2e/ts_library/some_module/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ nodejs_binary(
":main",
":some_module",
],
entry_point = "e2e_ts_library/some_module/main.js",
entry_point = ":main.ts",
)

sh_test(
Expand Down
23 changes: 3 additions & 20 deletions e2e/typescript_3.1/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "rollup_bundle")
load("@build_bazel_rules_nodejs//:defs.bzl", "rollup_bundle")
load("@npm_bazel_jasmine//:index.bzl", "jasmine_node_test")
load("@npm_bazel_typescript//:index.bzl", "ts_library")

Expand Down Expand Up @@ -57,34 +57,17 @@ jasmine_node_test(
],
)

nodejs_binary(
name = "main_js",
data = [
":main",
],
entry_point = "e2e_typescript_3_1/main.js",
)

nodejs_binary(
name = "main_js_sm",
data = [
":main",
"@npm//source-map-support",
],
entry_point = "e2e_typescript_3_1/main.js",
)

rollup_bundle(
name = "bundle",
entry_point = "main",
entry_point = ":main.ts",
deps = [
":main",
],
)

rollup_bundle(
name = "bundle_sm",
entry_point = "main",
entry_point = ":main.ts",
deps = [
":main",
"@npm//source-map-support",
Expand Down
2 changes: 1 addition & 1 deletion examples/app/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ ts_devserver(

rollup_bundle(
name = "bundle",
entry_point = "examples_app/app",
entry_point = ":app.ts",
deps = [":app"],
)

Expand Down
3 changes: 1 addition & 2 deletions examples/parcel/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ parcel(
name = "bundle",
srcs = [
"bar.js",
"foo.js",
],
entry_point = "foo.js",
entry_point = ":foo.js",
)

jasmine_node_test(
Expand Down
9 changes: 6 additions & 3 deletions examples/parcel/parcel.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ def _parcel_impl(ctx):
"""

# Options documented at https://parceljs.org/cli.html
args = ["build", ctx.attr.entry_point]
args = ["build", ctx.file.entry_point.short_path]
args += ["--out-dir", ctx.outputs.bundle.dirname]
args += ["--out-file", ctx.outputs.bundle.basename]

ctx.actions.run(
inputs = ctx.files.srcs,
inputs = ctx.files.srcs + [ctx.file.entry_point],
executable = ctx.executable.parcel,
outputs = [ctx.outputs.bundle, ctx.outputs.sourcemap],
arguments = args,
Expand All @@ -44,7 +44,10 @@ parcel = rule(
implementation = _parcel_impl,
attrs = {
"srcs": attr.label_list(allow_files = True),
"entry_point": attr.string(mandatory = True),
"entry_point": attr.label(
allow_single_file = True,
mandatory = True,
),
"parcel": attr.label(
# This default assumes that users name their install "npm"
default = Label("@npm//parcel-bundler/bin:parcel"),
Expand Down
2 changes: 1 addition & 1 deletion examples/program/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ nodejs_binary(
"index.js",
":node_modules",
],
entry_point = "examples_program/index.js",
entry_point = ":index.js",
)

jasmine_node_test(
Expand Down
2 changes: 1 addition & 1 deletion examples/protocol_buffers/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ ts_devserver(
# Test for production mode
rollup_bundle(
name = "bundle",
entry_point = "examples_protocol_buffers/app",
entry_point = ":app.ts",
# TODO(alexeagle): we should be able to get this from //:protobufjs_bootstrap_scripts
# and automatically plumb it through to Rollup.
globals = {
Expand Down
2 changes: 1 addition & 1 deletion examples/webapp/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ load("@build_bazel_rules_nodejs//internal/web_package:web_package.bzl", "web_pac
rollup_bundle(
name = "bundle",
srcs = glob(["*.js"]),
entry_point = "index.js",
entry_point = ":index.js",
)

web_package(
Expand Down
5 changes: 5 additions & 0 deletions internal/common/collect_es6_sources.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ def collect_es6_sources(ctx):
non_rerooted_files = [d for d in ctx.files.deps if d.is_source]
if hasattr(ctx.attr, "srcs"):
non_rerooted_files += ctx.files.srcs

# Some rules such as rollup_bundle specify an entry_point which should
# be collected if the file is a js file.
if hasattr(ctx.attr, "entry_point"):
non_rerooted_files += [s for s in ctx.files.entry_point if s.extension == "js"]
for dep in ctx.attr.deps:
if hasattr(dep, "typescript"):
non_rerooted_files += dep.typescript.transitive_es6_sources.to_list()
Expand Down
2 changes: 1 addition & 1 deletion internal/e2e/fine_grained_no_bin/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ nodejs_binary(
"test/test.js",
"@fine_grained_no_bin//fs.realpath",
],
entry_point = "build_bazel_rules_nodejs/internal/e2e/fine_grained_no_bin/index.js",
entry_point = ":index.js",
)
7 changes: 2 additions & 5 deletions internal/e2e/rollup/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@ load("//:defs.bzl", "rollup_bundle")

rollup_bundle(
name = "bundle",
srcs = [
"bar.js",
"foo.js",
],
entry_point = "internal/e2e/rollup/foo.js",
srcs = ["bar.js"],
entry_point = ":foo.js",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting that you don't have to specify foo.js in the srcs. It seems like it reduces one way users can fall off the happy path. on the other hand it means you have to consider entry_point when enumerating the inputs to a rule.
Should we give it more thought?

Copy link
Collaborator Author

@gregmagolan gregmagolan Jun 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. That's a good question. One strike against doing it this way is that if the user specifies :foo.ts as the entry_point they still have to add the ts_library :foo_lib to deps. I'm not sure what is best. On the other hand, the user adding :foo.js to entry_point and to srcs doesn't break anything with the current approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some smell however with not having the file in srcs as seen here: https://github.com/bazelbuild/rules_nodejs/pull/777/files#diff-6e8a939ed5da5eca956cd2f53701b77e. Probably better to force it to be specified

globals = {"some_global_var": "runtime_name_of_global_var"},
license_banner = ":license.txt",
deps = [
Expand Down
7 changes: 4 additions & 3 deletions internal/e2e/rollup_code_splitting/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ rollup_bundle(
["*.js"],
exclude = ["*.spec.js"],
),
additional_entry_points = ["internal/e2e/rollup_code_splitting/additional_entry.js"],
entry_point = "internal/e2e/rollup_code_splitting/main1.js",
additional_entry_points = ["build_bazel_rules_nodejs/internal/e2e/rollup_code_splitting/additional_entry.js"],
entry_point = ":main1.js",
license_banner = ":license.txt",
)

Expand All @@ -18,10 +18,11 @@ jasmine_node_test(
"main1.spec.js",
],
data = glob([
"*_golden.js_",
"goldens/*",
]) + [
":bundle",
":bundle.min.js",
":bundle.min.es2015.js",
],
deps = [
"//internal/e2e:check_lib",
Expand Down
3 changes: 2 additions & 1 deletion internal/e2e/rollup_code_splitting/additional_entry.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ const path = __dirname;

describe('bundling additional entry point', () => {
it('should work', () => {
check(path, 'bundle.min.js', 'bundle-min_golden.js_');
check(path, 'bundle.min.js', 'goldens/bundle.min.js_');
check(path, 'bundle.min.es2015.js', 'goldens/bundle.min.es2015.js_');
});

// Disabled because native ESModules can't be loaded in current nodejs
Expand Down
1 change: 1 addition & 0 deletions internal/e2e/rollup_code_splitting/goldens/bundle.min.es2015.js_
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import('./bundle_chunks_min_es2015/main1.js');
2 changes: 1 addition & 1 deletion ...llup_code_splitting/bundle-min_golden.js_ → ...lup_code_splitting/goldens/bundle.min.js_
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
(function(global) {
System.config({
packages: {
'': {map: {"./main1.js": "bundle_chunks_min/main1.js", "./additional_entry.js": "bundle_chunks_min/additional_entry.js"}, defaultExtension: 'js'},
'': {map: {"./main1": "bundle_chunks_min/main1", "./additional_entry.js": "bundle_chunks_min/additional_entry.js"}, defaultExtension: 'js'},
}
});
System.import('main1.js').catch(function(err) {
Expand Down
14 changes: 5 additions & 9 deletions internal/e2e/rollup_fine_grained_deps/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ load("//:defs.bzl", "jasmine_node_test", "nodejs_binary", "rollup_bundle")
# and no fine grained deps
rollup_bundle(
name = "bundle_no_deps",
srcs = ["no-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/no-deps.js",
entry_point = ":no-deps.js",
)

# You can have a rollup_bundle with no node_modules attribute
# and fine grained deps
rollup_bundle(
name = "bundle",
srcs = ["has-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
entry_point = ":has-deps.js",
deps = [
"@fine_grained_deps_yarn//@gregmagolan/test-a",
"@fine_grained_deps_yarn//@gregmagolan/test-b",
Expand All @@ -24,17 +22,15 @@ rollup_bundle(
# and no fine grained deps
rollup_bundle(
name = "bundle_legacy",
srcs = ["has-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
entry_point = ":has-deps.js",
node_modules = "@fine_grained_deps_yarn//:node_modules",
)

# You can have a rollup_bundle with both a node_modules attribute
# and fine grained deps so long as they come from the same root
rollup_bundle(
name = "bundle_hybrid",
srcs = ["has-deps.js"],
entry_point = "internal/e2e/rollup_fine_grained_deps/has-deps.js",
entry_point = ":has-deps.js",
node_modules = "@fine_grained_deps_yarn//:node_modules",
deps = [
"@fine_grained_deps_yarn//@gregmagolan/test-a",
Expand Down Expand Up @@ -93,6 +89,6 @@ nodejs_binary(
"@npm//jasmine",
"@npm//unidiff",
],
entry_point = "build_bazel_rules_nodejs/internal/e2e/rollup_fine_grained_deps/update_golden.js",
entry_point = ":update_golden.js",
install_source_map_support = False,
)
2 changes: 1 addition & 1 deletion internal/history-server/history_server.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def history_server(templated_args = [], **kwargs):

nodejs_binary_macro(
node_modules = "@history-server_runtime_deps//:node_modules",
entry_point = "history-server/modules/cli.js",
entry_point = "@history-server_runtime_deps//node_modules/history-server:modules/cli.js",
install_source_map_support = False,
templated_args = templated_args,
**kwargs
Expand Down
2 changes: 1 addition & 1 deletion internal/http-server/http_server.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def http_server(templated_args = [], **kwargs):

nodejs_binary_macro(
node_modules = "@http-server_runtime_deps//:node_modules",
entry_point = "http-server/bin/http-server",
entry_point = "@http-server_runtime_deps//node_modules/http-server:bin/http-server",
install_source_map_support = False,
templated_args = templated_args,
**kwargs
Expand Down
3 changes: 1 addition & 2 deletions internal/jasmine_node_test/jasmine_node_test.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ def jasmine_node_test(
)

all_data = data + srcs + deps
all_data += [Label("//internal/jasmine_node_test:jasmine_runner.js")]
all_data += [":%s_devmode_srcs.MF" % name]
entry_point = "build_bazel_rules_nodejs/internal/jasmine_node_test/jasmine_runner.js"
entry_point = Label("//internal/jasmine_node_test:jasmine_runner.js")

# If the target specified templated_args, pass it through.
templated_args = kwargs.pop("templated_args", []) + ["$(location :%s_devmode_srcs.MF)" % name]
Expand Down
2 changes: 1 addition & 1 deletion internal/jasmine_node_test/test/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ nodejs_test(
"no_jasmine_test.js",
"//internal/jasmine_node_test:jasmine_runner.js",
],
entry_point = "build_bazel_rules_nodejs/internal/jasmine_node_test/test/no_jasmine_test.js",
entry_point = ":no_jasmine_test.js",
)
Loading