Skip to content

Adopt new NodeJS runtime toolchain #2330

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adopt new NodeJS runtime toolchain #2330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ bazel_dep(name = "aspect_tools_telemetry", version = "0.2.8")
bazel_dep(name = "bazel_features", version = "1.9.0")
bazel_dep(name = "bazel_skylib", version = "1.5.0")
bazel_dep(name = "platforms", version = "0.0.5")
bazel_dep(name = "rules_nodejs", version = "6.3.0")
bazel_dep(name = "rules_nodejs", version = "6.6.0")

tel = use_extension("@aspect_tools_telemetry//:extension.bzl", "telemetry")
use_repo(tel, "aspect_tools_telemetry_report")
Expand Down Expand Up @@ -58,10 +58,10 @@ bazel_dep(name = "rules_shell", version = "0.6.1", dev_dependency = True)
# Overrides of non-dev deps for local testing using recent versions
archive_override(
module_name = "rules_nodejs",
integrity = "sha256-OY23vNcf28WAxvjReCrpn1JQeRFypf/jHS+dgTjzOC4=",
strip_prefix = "rules_nodejs-7cbb1fb5378fb4739e4d9698a14abd9eada900f0",
integrity = "sha256-jFmNXCy0m8FjDy2Je31l1PC/kvDq3J3TdNhZ3+mojUs=",
strip_prefix = "rules_nodejs-2d3b73e0596924e5798df464ebc5d85463ac6a75",
urls = [
"https://github.com/bazel-contrib/rules_nodejs/archive/7cbb1fb5378fb4739e4d9698a14abd9eada900f0.tar.gz",
"https://github.com/bazel-contrib/rules_nodejs/archive/2d3b73e0596924e5798df464ebc5d85463ac6a75.tar.gz",
],
)

Expand Down
2 changes: 1 addition & 1 deletion e2e/pnpm_workspace/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ sh_test(
"@nodejs_linux_amd64//:node_files",
"@nodejs_linux_arm64//:node_files",
],
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
toolchains = ["@rules_nodejs//nodejs:current_node_runtime"],
)

build_test(
Expand Down
2 changes: 1 addition & 1 deletion e2e/pnpm_workspace_rerooted/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ sh_test(
"@nodejs_linux_amd64//:node_files",
"@nodejs_linux_arm64//:node_files",
],
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
toolchains = ["@rules_nodejs//nodejs:current_node_runtime"],
)

build_test(
Expand Down
8 changes: 4 additions & 4 deletions examples/genrule/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ genrule(
# $@ is bazel shorthand for the path of the output file
">$@",
]),
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
tools = ["@nodejs_toolchains//:resolved_toolchain"],
toolchains = ["@rules_nodejs//nodejs:current_node_toolchain"],
tools = ["@rules_nodejs//nodejs:current_node_toolchain"],
)

diff_test(
Expand Down Expand Up @@ -74,8 +74,8 @@ genrule(
$(NODE_PATH) \\
./$(execpath :require_acorn_js) \\
$@""",
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
tools = ["@nodejs_toolchains//:resolved_toolchain"],
toolchains = ["@rules_nodejs//nodejs:current_node_toolchain"],
tools = ["@rules_nodejs//nodejs:current_node_toolchain"],
)

diff_test(
Expand Down
6 changes: 3 additions & 3 deletions js/private/js_binary.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ _ATTRS = {
""",
),
"node_toolchain": attr.label(
doc = """The Node.js toolchain to use for this target.
doc = """The Node.js runtime toolchain to use for this target.

See https://bazel-contrib.github.io/rules_nodejs/Toolchains.html

Expand Down Expand Up @@ -505,7 +505,7 @@ def _create_launcher(ctx, log_prefix_rule_set, log_prefix_rule, fixed_args = [],
if ctx.attr.node_toolchain:
nodeinfo = ctx.attr.node_toolchain[platform_common.ToolchainInfo].nodeinfo
else:
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:runtime_toolchain_type"].nodeinfo

if DirectoryPathInfo in ctx.attr.entry_point:
entry_point = ctx.attr.entry_point[DirectoryPathInfo].directory
Expand Down Expand Up @@ -636,7 +636,7 @@ js_binary_lib = struct(
toolchains = [
# TODO: on Windows this toolchain is never referenced
"@bazel_tools//tools/sh:toolchain_type",
"@rules_nodejs//nodejs:toolchain_type",
"@rules_nodejs//nodejs:runtime_toolchain_type",
] + COPY_FILE_TO_BIN_TOOLCHAINS,
)

Expand Down
7 changes: 3 additions & 4 deletions js/private/js_image_layer.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ else {
transitive = [files],
)

nodeinfo = ctx.attr._current_node[platform_common.ToolchainInfo].nodeinfo
nodeinfo = ctx.attr._node_tool[platform_common.ToolchainInfo].nodeinfo
if hasattr(nodeinfo, "node"):
node_exec = nodeinfo.node
else:
Expand Down Expand Up @@ -627,8 +627,8 @@ js_image_layer_lib = struct(
default = "//js/private:js_image_layer.mjs",
allow_single_file = True,
),
"_current_node": attr.label(
default = "@nodejs_toolchains//:resolved_toolchain",
"_node_tool": attr.label(
Copy link
Contributor

Choose a reason for hiding this comment

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

This attribute could be replaced with @rules_nodejs//nodejs:toolchain_type as that matches on the exec platform.

Copy link
Author

@guw guw Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What is the recommended approach?

I could replace:

    nodeinfo = ctx.attr._node_tool[platform_common.ToolchainInfo].nodeinfo
    ....

with:

    nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
    if hasattr(nodeinfo, "node"):
        node_exec = nodeinfo.node
    else:
        # TODO(3.0): drop support for deprecated toolchain attributes
        node_exec = nodeinfo.target_tool_path
    ctx.actions.run(
        inputs = inputs,
        arguments = [splitter.path],
        unused_inputs_list = unused_inputs,
        outputs = splitter_outputs,
        executable = node_exec,
        progress_message = "Computing Layer Groups %{label}",
        mnemonic = "JsImageLayerGroups",
    )

But how do I control selection based on exec then?

Copy link
Author

@guw guw Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Oh so, nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo would always match the execution platform?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, since that's what we ask the toolchains registered for this type to do. That's the idiomatic approach to use tools.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just keep in mind that the ctx.actions.run call should set the toolchain parameter to the toolchain type used in the action (if there is a single one, which is the case here).

default = "@rules_nodejs//nodejs:current_node_toolchain",
cfg = "exec",
),
"binary": attr.label(
Expand Down Expand Up @@ -687,6 +687,5 @@ js_image_layer = rule(
doc = _DOC,
toolchains = [
tar_lib.toolchain_type,
"@rules_nodejs//nodejs:toolchain_type",
],
)
2 changes: 1 addition & 1 deletion npm/private/test/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ sh_test(
"@nodejs_linux_amd64//:node_files",
"@nodejs_linux_arm64//:node_files",
],
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
toolchains = ["@rules_nodejs//nodejs:current_node_runtime"],
)