5.0.0 release candidates feedback

[alexeagle]

Here it is, our twice-yearly major release with breaking changes and big improvements.

The migration guide is at https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0

---

This discussion was created from the release 5.0.0-rc.0.

[duarten]

I'm having trouble getting this to work. I'm not sure if the upgrade guide refers to "build_bazel_rules_nodejs" or just to the core package.

The build_bazel_rules_nodejs package doesn't have any nodejs/repositories.bzl file. It does have a top-level repositories.bzl file, exposing a build_bazel_rules_nodejs_dependencies macro instead of rules_nodejs_dependencies. But then I can't find any nodejs_register_toolchains macro.

[alexeagle]

yeah it should have been

load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")

build_bazel_rules_nodejs_dependencies()

it changed between initially writing the wiki page and the final rc0 release, I'll update on the wiki

[duarten]

What should the rest of the WORKSPACE look like? I get

ERROR: error loading package '': in /private/var/tmp/_bazel_duarte.nunes/212761d3caf5ba6776dcf84677e3f64d/external/build_bazel_rules_nodejs/index.bzl: in /private/var/tmp/_bazel_duarte.nunes/212761d3caf5ba6776dcf84677e3f64d/external/build_bazel_rules_nodejs/internal/npm_install/npm_install.bzl: cannot load '@rules_nodejs//nodejs/private:node_labels.bzl': no such file

with

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "build_bazel_rules_nodejs",
    sha256 = "5b2909656af044442d206ad3609b09d456d1edd80f90162640e608ae5408b0fb",
    urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/5.0.0-rc.0/rules_nodejs-5.0.0-rc.0.tar.gz"],
)

load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")

build_bazel_rules_nodejs_dependencies()

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")

node_repositories(
    package_json = ["//:package.json"],
)

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

[alexeagle]

sorry, rc.0 was busted, it had a 4.4.6 version of the nested dep that should have been 5

use rc.1 to fix that

[alexeagle]

and rc.1 was missing tar.gz artifacts... just fixed

[duarten]

ah-ha, thanks! update now went smoothly; great work! :)

[dae]

Testing with rc1, DeclarationInfo seems to be missing from @rules_nodejs//:providers.bzl - it seems like that file is the same one as in the @build_bazel_rules_nodejs repo:

% diff /private/var/tmp/_bazel_dae/4928556feb6f3e9c4d256de8caa46cdd/external/{build_bazel_rules_nodejs,rules_nodejs}/providers.bzl
%

Edit: ah, it seems I should be using @rules_nodejs//nodejs:providers.bzl

[alexeagle]

Yeah that file shouldn't be in the distro. Will fix...

[alexeagle]

Fixed for rc.2

[dae]

I was excited to see the change to handling npm deps as directories, as the plethora of symlinks in each build step / runfiles can be painful. This seems to work well in some cases, but I'm seeing cases where each individual file in a directory is symlinked in like before, instead of the directory itself. For example, given the following genrule:

genrule(
    name = "mytest",
    srcs = [
        "@npm//tslib",
    ],
    outs = ["mytest.out"],
    cmd = "pwd; ls -ld $<; echo '***'; ls -l $<",
)

When run, it shows:

INFO: From Executing genrule //ts/card-info:mytest:
/private/var/tmp/_bazel_dae/4928556feb6f3e9c4d256de8caa46cdd/sandbox/darwin-sandbox/1126/execroot/ankidesktop
drwxr-xr-x  12 dae  wheel  384 Jan 17 16:54 bazel-out/darwin_arm64-fastbuild/bin/external/npm/node_modules/tslib
***
total 0
lrwxr-xr-x  1 dae  wheel  170 Jan 17 16:54 CopyrightNotice.txt -> /private/var/tmp/_bazel_dae/4928556feb6f3e9c4d256de8caa46cdd/execroot/ankidesktop/bazel-out/darwin_arm64-fastbuild/bin/external/npm/node_modules/tslib/CopyrightNotice.txt
lrwxr-xr-x  1 dae  wheel  162 Jan 17 16:54 LICENSE.txt -> /private/var/tmp/_bazel_dae/4928556feb6f3e9c4d256de8caa46cdd/execroot/ankidesktop/bazel-out/darwin_arm64-fastbuild/bin/external/npm/node_modules/tslib/LICENSE.txt
[...]

Is that expected behaviour?

Also, it looks like npm_sass_library() is also incompatible with this directory change. I presumably can hack around it for now by adding node_modules to include_paths. (As an aside, rules_sass seems to trip me up each time a new rules_nodejs comes out. The repo seems fairly quiet - are people using some other solution for sass building?)

[alexeagle]

For rules_sass - I agree it has some tight coupling with rules_nodejs. I think they're accepting changes though.
I'd just use the sass rule generated for the https://www.npmjs.com/package/sass package, without rules_sass. That's more like how you'd invoke any other sass preprocessor like less or stylus, as we do in our examples.

[alexeagle]

To see the inputs to the action, run bazel aquery //ts/card-info:mytest to see the single input artifact being provided.

The content of that folder being symlinks does surprise me though. Maybe @gregmagolan has a satisfying answer.

[dae]

[edit: sorry, I misread your comment and ended up repeating what you said]