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

Node 20 compatibility #130

Closed

Conversation

Nebulous-Narwhal-48
Copy link

Fixed server not running with newer node versions.
Updated uws and pino versions and fixed some breaking changes.
I tested only node 20

@wight-airmash
Copy link
Owner

Sorry for the long response.

Upgrading the node version requires performance tests, especially those that carry a backward incompatibility like this. I refused to upgrade to versions 14 and 16 back in the day due to significant performance drops. V8 also brought changes and optimisations during the time from node 12, which requires server refactoring.

I haven't seen any further changes in your repository that would require version 20 of node, there's only the upgrade itself. Why do you want to upgrade to version 20 specifically?

@Nebulous-Narwhal-48
Copy link
Author

20 is just the last lts release. Upgrading would make local dev and deploy less painful, without relying on docker. But I totally understand your reservations.

@wight-airmash
Copy link
Owner

Yeah, I see. But in this case, convenience of development < server performance. And the changes after node 12.16.3 added pain in upgrading process. In practice, game loop latency doubled on versions 14 and 16, this is simply unacceptable, and the reason for this performance regression hasn't been fixed in the next versions of node (and probably won't be at all). Benchmarking isn't the strongest part of node (just for example, recent release-related issue with pretty common operations).

You probably know about NVM, but I leave it here anyway just in case. In my experience on linux/macos it works fine out of the box even with older versions of node.

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.

2 participants