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 for node v18 #1143

Closed
wants to merge 4 commits into from
Closed

Update for node v18 #1143

wants to merge 4 commits into from

Conversation

csillag
Copy link
Contributor

@csillag csillag commented Nov 11, 2022

In the current version of the code

  • The web wallet can be compiled with both node v16 and v18
  • The extension can only be built with v16; with v18, parcel hits some errors.

This is an upstream bug that has been fixed since we locked our dependency versions.
The situation can be resolved by removing yarn.lock and thus updating all our dependencies.

This results in many changes.
One of the changes is that date formatting now uses a different kind of space; I updated the tests for that.

After this changes node v18 works OK (compilation, tests, everything), but node v14 and v16 no longer works.
(Actually, the code does work with node v16, but the tests don't, and I didn't feel like debugging it, since node v18 is already the current LTS release...)

So of we do this, everyone working on the wallet must upgrade to node v18.

@github-actions
Copy link

github-actions bot commented Nov 11, 2022

🦙 MegaLinter status: ❌ ERROR

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.12s
✅ BASH bash-exec 1 0 0.0s
✅ BASH shellcheck 1 0 0.02s
✅ BASH shfmt 1 0 0 0.01s
✅ CSS stylelint 1 0 0 0.89s
✅ EDITORCONFIG editorconfig-checker 111 0 0.16s
✅ HTML htmlhint 1 0 0.22s
✅ JAVASCRIPT eslint 9 0 0 3.32s
✅ JSON eslint-plugin-jsonc 6 0 0 0.85s
✅ JSON jsonlint 6 0 0.21s
❌ JSON npm-package-json-lint yes 1 0.46s
✅ JSON prettier 6 0 0 0.62s
✅ JSON v8r 6 0 8.4s
✅ REPOSITORY checkov yes no 12.47s
✅ REPOSITORY git_diff yes no 0.0s
✅ TSX eslint 29 0 0 7.96s
✅ TYPESCRIPT eslint 36 0 0 8.54s
✅ YAML prettier 4 0 0 1.31s
✅ YAML v8r 4 0 2.78s
✅ YAML yamllint 4 0 0.27s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@csillag csillag changed the title Update dependencies so that we can compile with node v18 Update for node v18 Nov 11, 2022
@csillag
Copy link
Contributor Author

csillag commented Nov 11, 2022

Please note that the failing tests are because the CI is still configured to use node v14, and our package json demands v18.

@csillag csillag marked this pull request as ready for review November 11, 2022 11:04
@csillag
Copy link
Contributor Author

csillag commented Nov 11, 2022

As an aside, one could probably do a tighter update; ie. instead of removing the whole yarn.lock file, update only a few dependencies selectively... but it's a PITA to figure out exactly which ones, and eventually we need to upgrade all of them anyway.

@buberdds
Copy link
Contributor

buberdds commented Nov 11, 2022

Can you bump node version in GH actions? .github/workflows search for 14.x.

@csillag csillag self-assigned this Nov 11, 2022
@csillag csillag marked this pull request as draft November 11, 2022 22:41
@csillag
Copy link
Contributor Author

csillag commented Nov 11, 2022

There is something wrong with the newer react dependencies...

@csillag csillag force-pushed the csillag/update-parcel branch 2 times, most recently from 7439e7c to e812396 Compare November 12, 2022 00:33
@csillag
Copy link
Contributor Author

csillag commented Nov 12, 2022

There is something wrong with the newer react dependencies...

That has been solved, but now I have an issue whereas Intl.DateTimeFormat behaves differently locally and on GitHub. (Although node v18.12.x is running at both configurations)

@csillag csillag force-pushed the csillag/update-parcel branch 2 times, most recently from 0546571 to 27d7d3c Compare November 12, 2022 01:26
@csillag
Copy link
Contributor Author

csillag commented Nov 12, 2022

Now only the Cypress tests are dead...

This change has been created my removing yarn.lock,
and then running yarn again to resolve all the dependencies.

Lots of versions change.
The new Intl.dateFormat functions sometimes use a
"Narrow No-Break Space" instead of a normal space
in the dates, before the AM/PM suffices.

The behavior seems to be somewhat non-deterministic, which
makes testing difficult.

This change simply normalizes the output to always use a simple space.
@cloudflare-pages
Copy link

cloudflare-pages bot commented Mar 3, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7afdd34
Status:🚫  Build failed.

View logs

@lukaw3d lukaw3d mentioned this pull request Mar 3, 2023
@buberdds
Copy link
Contributor

Done via #1306

@buberdds buberdds closed this Mar 16, 2023
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.

None yet

3 participants