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

Upgrade dependencies (fixes errors with modern Node) #186

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

asmello
Copy link

@asmello asmello commented May 7, 2023

I've found that the current template does not work with Node 17+, due to this break. A fix for that is available in more recent versions of Webpack, so bumping the dependency version is a valid fix for the issue. Of course, since the Webpack upgrade is itself breaking, I've had to upgrade other dependencies as well, as well as modify some configuration files.

Since most dependencies were very stale and I was getting a lot of warnings for deprecations and vulnerabilities, I decided to go for a full upgrade of everything. This includes both JS and Rust dependencies, and even the Rust edition, which I bumped to 2021. Small changes to sample code were needed. As a bonus, I got rid of the dependency on the futures crate, as native futures are now supported.

One side change I did after going over warnings was to remove the [target."cfg(debug_assertions)".dependencies] block. As reported here (and documented here) it doesn't actually do anything, just gets silently ignored. Removing that bug makes sense as it's something that is now reported with newer Rust.

@asmello
Copy link
Author

asmello commented May 7, 2023

Note this is overlapping with other PRs like #182, but more comprehensive.

@nickgirardo
Copy link

Super helpful pr! I just had one nit regarding the webpack config. If I wasn't clear please feel free to reply!

@asmello
Copy link
Author

asmello commented Jul 4, 2023

BTW I didn't touch the parent script but it's also quite stale (it's got some very old versions on the lockfile, and the travis setup is relying on Node 10).

@feynon
Copy link

feynon commented Mar 2, 2024

What seems to be the blocker for this PR to get merged?

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