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 #1135

Merged
merged 43 commits into from
Oct 23, 2023
Merged

Upgrade dependencies #1135

merged 43 commits into from
Oct 23, 2023

Conversation

hugojosefson
Copy link
Contributor

@hugojosefson hugojosefson commented Oct 15, 2023

This builds upon #1134 by @daanh432, which uses Node.js 18.
In this PR, I upgrade (almost) all other dependencies, for security, and for trying to stay up-to-date.

For some dependencies, such as typescript v4 → v5, the amount of things to fix after such an upgrade was too great for me at the moment, so I stayed on the latest minor version I could successfully upgrade to.

Some minor, and some major, version upgrades to dependencies required small updates in code, which I included in each such upgrade commit together with the dependency version bump.

I successfully ran this command, at least after every major version upgrade commit, and at the end:

npm install \
&& npm run lint \
&& bash -euo pipefail ./scripts/tsc.sh \
&& npm test \
&& npm run e2e -- --fail-fast \
&& npm run e2e:uws -- --fail-fast \
&& bash scripts/package.sh true true \
&& node scripts/node-test.js \
&& node scripts/executable-test.js

That way, I feel confident this should work.

I have also tested the major upgrade of commander manually with the built executable ./build/deepstream, to make sure it sees the start command as the default and chooses the correct command when given another one.

I updated one test to make it work correctly in both Node.js 18 and Node.js 20, because the error message it checks for, becomes longer and more specific in Node.js 20.


Build is green on my fork's GitHub Actions: https://github.com/hugojosefson/deepstream.io/actions/runs/6591970600

daanh432 and others added 30 commits September 9, 2023 17:26
Using corresponding new arguments, like the warnings recommended.
Upgrade some deps to automatically fix their security issues.
Could not find this depencency used of referenced anywhere.
Could not find this depencency used of referenced anywhere.
Latest version 4 for now, because it requires the least changes to existing code and types.
@@ -291,7 +291,7 @@ export const validate = function (config: Object): void {
const valid = validator(config)

if (!valid) {
const output = (betterAjvErrors(schema, config, validator.errors, { format: 'js' }) as never as betterAjvErrors.IOutputError[])
const output = betterAjvErrors(schema, config, validator.errors ?? [], { format: 'js' })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new version returns the expected type, thanks to { format: 'js' } in the options, so no cast is needed.

@hugojosefson hugojosefson marked this pull request as ready for review October 15, 2023 14:54
@jaime-ez
Copy link
Collaborator

Awesome, I'll check the change and leave some comments

If my calculations are correct, "without without no" simplifies to "without".
@jaime-ez jaime-ez merged commit 3e459ff into deepstreamIO:master Oct 23, 2023
1 check passed
@hugojosefson hugojosefson deleted the upgrade-deps branch October 23, 2023 21:13
@hugojosefson
Copy link
Contributor Author

Thank you @jaime-ez for taking the time to review and merge this!

@jaime-ez
Copy link
Collaborator

Thanks @daanh432 and @hugojosefson for your PRs!
Awesome to see some actual work on the project from others.
new release will be available soon, please test and inform any issues.

Cheers

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