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

build: upgrade to use nodev20 #11410

Merged
merged 5 commits into from
Jul 27, 2023
Merged

build: upgrade to use nodev20 #11410

merged 5 commits into from
Jul 27, 2023

Conversation

chenrui333
Copy link
Contributor

👋 node 16 is gonna be EOL on 11 Sep 2023, right now, building with node v20 would have digital envelope routines::unsupported issue, thus updating the build to use v20.

closes #11404
relates to Homebrew/homebrew-core#137118

cc @templarfelix @JPZ13

Signed-off-by: Rui Chen <rui@chenrui.dev>
@terrytangyuan
Copy link
Member

terrytangyuan commented Jul 21, 2023

I remember you had a comment on the node version before, right? @jmeridth

@terrytangyuan
Copy link
Member

@tczhao @JPZ13 Could you help review this?

@jmeridth
Copy link
Member

@terrytangyuan was regarding #10126

my issue was that I was able to get a feature working with v18 and then we (argo-workflows) went back to v16. Feature broke. Going to v20 would be awesome.

@terrytangyuan
Copy link
Member

Thanks.

Please fix the CI build failures.

Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

A few other places we should update too, like the Nixfiles in dev/nix/flake.nix.
EDIT: actually in flake.nix not node-env.nix, my bad. See below comments

@@ -6,7 +6,7 @@
"src"
],
"scripts": {
"build": "rm -Rf dist && NODE_OPTIONS='' NODE_ENV=production webpack --mode production --config ./src/app/webpack.config.js",
"build": "rm -Rf dist && NODE_OPTIONS='--openssl-legacy-provider' NODE_ENV=production webpack --mode production --config ./src/app/webpack.config.js",
"start": "NODE_OPTIONS='--no-experimental-fetch' webpack-dev-server --config ./src/app/webpack.config.js",
"lint": "tslint --fix -p ./src/app",
"test": "jest"
Copy link
Contributor

Choose a reason for hiding this comment

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

on line 15 below, engines.node should be updated as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me add that.

Signed-off-by: Rui Chen <rui@chenrui.dev>
@chenrui333
Copy link
Contributor Author

A few other places we should update too, like the Nixfiles in dev/nix/node-env.nix

do you have any guide about it?

@chenrui333 chenrui333 changed the title deps: upgrade to use nodev20 build: upgrade to use nodev20 Jul 26, 2023
Signed-off-by: Rui Chen <rui@chenrui.dev>
@agilgur5
Copy link
Contributor

do you have any guide about it?

My bad, it's actually in flake.env. My Nix is very rusty but I can pull up something tomorrow probably.

If you're not sure, I'm ok approving without so long as CI passes. This is a good change to have and we can update Nix in a follow-up PR 🙂

@isubasinghe
Copy link
Member

@agilgur5 I can push the Nix stuff, I was the one who ported the build process over to Nix.
Should be a simple 5 min job on my end.

@isubasinghe
Copy link
Member

isubasinghe commented Jul 26, 2023

@terrytangyuan I think the build issues are not related to this PR.
I've got the same exact issues in my PR, likely flaky tests.

EDIT:
There are other issues as @alexec points out below.

@alexec
Copy link
Contributor

alexec commented Jul 26, 2023

Build error is this: "Error: error:0308010C:digital envelope routines::unsupported"

@alexec
Copy link
Contributor

alexec commented Jul 26, 2023

image

ui/package.json Outdated Show resolved Hide resolved
@alexec
Copy link
Contributor

alexec commented Jul 26, 2023

@isubasinghe have you considered presenting on Nix to community meeting? Nix is new hotness.

Signed-off-by: Alex Collins <alexec@users.noreply.github.com>
@isubasinghe
Copy link
Member

@alexec unfortunately my timezone makes attending the community meetings near impossible :(

@terrytangyuan terrytangyuan merged commit d876464 into argoproj:master Jul 27, 2023
22 checks passed
@terrytangyuan
Copy link
Member

terrytangyuan commented Jul 27, 2023

https://github.com/argoproj/argo-workflows/actions/runs/5676115276/job/15382363976

error argo-ui@1.0.0: The engine "node" is incompatible with this module. Expected version ">=20". Got "18.17.0"

Would anyone like to help fix that CI failure?

@terrytangyuan
Copy link
Member

Tracking in #11459

@agilgur5
Copy link
Contributor

Ah, this is from the snyk workflow, not ci-build. I'll make a PR for that

@agilgur5
Copy link
Contributor

agilgur5 commented Jul 28, 2023

Nix is new hotness.

2 decades for some more mainstream attention 😅

@alexec unfortunately my timezone makes attending the community meetings near impossible :(

I have yet to go to one but I imagine you could pre-record something which could just be played during the meeting?
@isubasinghe Also regarding NPM, did you use node2nix to generate the Nix expressions? It looks vaguely similar, but docs don't mention it. Last time I used Nix, I think NPM didn't exist yet 😅 (back when Node was still up-and-coming and the iojs fork happened etc etc)

@isubasinghe
Copy link
Member

@agilgur5 I did use node2nix, its a bit of a manual process to use it with flakes however.
Probably not the best actually, I am planning to rewrite it using mkDerivation.
node2nix was just a quicker way of getting something up.

Also yeah I think I could probably pre-record something instead, will talk to someone about this later.

terrytangyuan added a commit that referenced this pull request Aug 11, 2023
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Alex Collins <alexec@users.noreply.github.com>
Co-authored-by: Yuan Tang <terrytangyuan@gmail.com>
Co-authored-by: Alex Collins <alexec@users.noreply.github.com>
terrytangyuan pushed a commit that referenced this pull request Aug 21, 2023
Signed-off-by: isubasinghe <isitha@pipekit.io>
@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies area/build Build or GithubAction/CI issues javascript Pull requests that update Javascript dependencies labels Aug 31, 2023
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Alex Collins <alexec@users.noreply.github.com>
Co-authored-by: Yuan Tang <terrytangyuan@gmail.com>
Co-authored-by: Alex Collins <alexec@users.noreply.github.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/ui javascript Pull requests that update Javascript dependencies type/dependencies PRs and issues specific to updating dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

digital envelope routines::unsupported issue with 3.4.9 release build
6 participants