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

Update NodeJS Versions #3319

Merged
merged 1 commit into from
Feb 13, 2022
Merged

Update NodeJS Versions #3319

merged 1 commit into from
Feb 13, 2022

Conversation

mattem
Copy link
Collaborator

@mattem mattem commented Feb 9, 2022

  • Updated NodeJS Versions using yarn run update-nodejs-versions

Auto-generated by create-pull-request

@mattem
Copy link
Collaborator Author

mattem commented Feb 13, 2022

Thanks!

@mattem mattem merged commit 387aad1 into stable Feb 13, 2022
@mattem mattem deleted the update-nodejs-versions branch February 13, 2022 01:05
@mitul45
Copy link

mitul45 commented Feb 24, 2022

Hey @mattem @alexeagle, I want to use node 16.14.0 in one of the project but I see that this PR was not included in the latest release of v4.6.2. Is that a reason for it?

I want to use 16.14.0 because I want this patch included.

@mattem
Copy link
Collaborator Author

mattem commented Feb 24, 2022

Not sure why the other version bump diff was included. Either way, you can set other versions of nodejs via the node_repositories attr, docs here

@mitul45
Copy link

mitul45 commented Feb 24, 2022

that's exactly what I am trying but I get following:

Error in fail: yarn_install failed: yarn install v1.22.11
[1/5] Validating package.json...
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
 (error nodejs@0.0.1: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.17.5" error Found incompatible module.)

And I assume, that 14.17.5 is because of this line.

@mitul45
Copy link

mitul45 commented Feb 24, 2022

I also have a sample playground here: https://github.com/mitul45/bazel-example/.

For example, if you checkout to 949c9ca54712e689681394623dd96f41255de18b, and run yarn bazel run :bin, you would see 14.17.5 as output while the node_version defined in WORKSPACE is 16.14.0.

But if you checkout to 4317f4bb1b0f108b4cc9f6019afb38c778016d94 which uses 16.13.0 in WORKSPACE – running the app prints correct 16.13.0 version.

@mattem
Copy link
Collaborator Author

mattem commented Feb 25, 2022

Ah, I think I see the bug, it's due to node_exists_for_os not taking into account the custom Node repositories:
https://github.com/bazelbuild/rules_nodejs/blob/stable/nodejs/private/os_name.bzl#L75

I'll put up a PR to fix.

You're example repo was helpful, thanks! You still need to set node_repositories though, the URLs alone are not enough.

node_repositories(
    node_version = "16.14.0",
    node_repositories = {
        "16.14.0-darwin_arm64": ("node-v16.14.0-darwin-arm64.tar.gz", "node-v16.14.0-darwin-arm64", "56e547d22bc7be8aa40c8cfd604c156a5bcf8692f643ec1801c1fa2390498542"),
        "16.14.0-darwin_amd64": ("node-v16.14.0-darwin-x64.tar.gz", "node-v16.14.0-darwin-x64", "26702ab17903ad1ea4e13133bd423c1176db3384b8cf08559d385817c9ca58dc"),
        "16.14.0-linux_arm64": ("node-v16.14.0-linux-arm64.tar.xz", "node-v16.14.0-linux-arm64", "5a6e818c302527a4b1cdf61d3188408c8a3e4a1bbca1e3f836c93ea8469826ce"),
        "16.14.0-linux_ppc64le": ("node-v16.14.0-linux-ppc64le.tar.xz", "node-v16.14.0-linux-ppc64le", "d1012696cacb3833b8b33748905e716f2524766b29a2a4c405f34ed2f3e5fdb4"),
        "16.14.0-linux_s390x": ("node-v16.14.0-linux-s390x.tar.xz", "node-v16.14.0-linux-s390x", "1c98494bc097547bcadb972780aec58bb40b9c094f0daed75debfee4cb980fd9"),
        "16.14.0-linux_amd64": ("node-v16.14.0-linux-x64.tar.xz", "node-v16.14.0-linux-x64", "0570b9354959f651b814e56a4ce98d4a067bf2385b9a0e6be075739bc65b0fae"),
        "16.14.0-windows_amd64": ("node-v16.14.0-win-x64.zip", "node-v16.14.0-win-x64", "c783f32aa22758e9fdcabb23daf348cc52f876fbd679d54edc2c4921ccd6d6c5"),
    },
    yarn_version = "1.22.4",
)

@mitul45
Copy link

mitul45 commented Feb 25, 2022

Ahh okay, just to clarify adding node_repositories won't fix the issue till you take care of node_exists_for_os function, right?

Because I'm still getting 14.17.5 after defining node_repositories.

$ git diff
diff --git a/WORKSPACE b/WORKSPACE
index a81d5fd..2052d97 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -18,8 +18,17 @@ http_archive(

 load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories")
 node_repositories(
-    node_version = "16.13.0",
+    node_version = "16.14.0",
     yarn_version = "1.22.4",
+    node_repositories = {
+      "16.14.0-darwin_arm64": ("node-v16.14.0-darwin-arm64.tar.gz", "node-v16.14.0-darwin-arm64", "56e547d22bc7be8aa40c8cfd604c156a5bcf8692f643ec1801c1fa2390498542"),
+      "16.14.0-darwin_amd64": ("node-v16.14.0-darwin-x64.tar.gz", "node-v16.14.0-darwin-x64", "26702ab17903ad1ea4e13133bd423c1176db3384b8cf08559d385817c9ca58dc"),
+      "16.14.0-linux_arm64": ("node-v16.14.0-linux-arm64.tar.xz", "node-v16.14.0-linux-arm64", "5a6e818c302527a4b1cdf61d3188408c8a3e4a1bbca1e3f836c93ea8469826ce"),
+      "16.14.0-linux_ppc64le": ("node-v16.14.0-linux-ppc64le.tar.xz", "node-v16.14.0-linux-ppc64le", "d1012696cacb3833b8b33748905e716f2524766b29a2a4c405f34ed2f3e5fdb4"),
+      "16.14.0-linux_s390x": ("node-v16.14.0-linux-s390x.tar.xz", "node-v16.14.0-linux-s390x", "1c98494bc097547bcadb972780aec58bb40b9c094f0daed75debfee4cb980fd9"),
+      "16.14.0-linux_amd64": ("node-v16.14.0-linux-x64.tar.xz", "node-v16.14.0-linux-x64", "0570b9354959f651b814e56a4ce98d4a067bf2385b9a0e6be075739bc65b0fae"),
+      "16.14.0-windows_amd64": ("node-v16.14.0-win-x64.zip", "node-v16.14.0-win-x64", "c783f32aa22758e9fdcabb23daf348cc52f876fbd679d54edc2c4921ccd6d6c5"),
+    },
     node_urls = [
         "https://nodejs.org/dist/v{version}/{filename}",
     ],


$ ./node_modules/@bazel/bazelisk/bazelisk-darwin_amd64 run :bin
INFO: Analyzed target //:bin (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:bin up-to-date:
  bazel-bin/bin.sh
  bazel-bin/bin_loader.js
  bazel-bin/bin_require_patch.js
INFO: Elapsed time: 0.398s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
v14.17.5

@mattem
Copy link
Collaborator Author

mattem commented Feb 25, 2022

Yes, correct, we'll need the fix to node_exists_for_os before any of this works

@mattem
Copy link
Collaborator Author

mattem commented Feb 25, 2022

This got released at 4.6.3. Thanks for the report and simple repro!

@mitul45
Copy link

mitul45 commented Feb 28, 2022

Thank you for the quick fix and a release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants