-
Notifications
You must be signed in to change notification settings - Fork 19
Update node 22 #1132
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 node 22 #1132
Conversation
alexcos20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
alexcos20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI automated code review (Gemini 3).
Overall risk: medium
Summary:
This pull request updates the Node.js version from v20 to v22 across the project, including CI workflows, .nvmrc, and package.json dependencies. It also updates the JSON import syntax from assert { type: 'json' } to with { type: 'json' } in numerous files. Additionally, it includes an important fix for indexer worker thread management to ensure only one worker runs per chain ID and adds better test environment cleanup.
Comments:
• [WARNING][other] The Node.js version has been updated from v20.19.0 to v22.5.1. This is a major version bump and while package-lock.json has been updated, please ensure that all production dependencies are fully compatible with Node.js v22, as there might be subtle breaking changes not caught by existing tests. A thorough integration test suite would be crucial here.
• [INFO][style] Updating .nvmrc to 22 is good practice, aligning with the ci.yml and package.json Node.js version changes. This helps ensure consistent Node.js versions across development and CI environments.
• [INFO][other] Updates to @types/node, prettier, and typescript are good to keep dependencies current and aligned with the new Node.js version. The addition of typescript to devDependencies is also a positive step for consistent type checking.
• [INFO][other] The extensive changes in package-lock.json are expected due to the Node.js version upgrade and dependency bumps. This should be verified by running npm install and ensuring all local development environments reflect these changes for consistency.
• [INFO][style] Minor formatting adjustment for TypesenseCollectionUpdateSchema improves readability. Good for consistency.
• [INFO][bug] Introducing globalWorkers and explicitly stopping existing worker threads for a given chain ID before starting a new one in startThread is a crucial improvement. This prevents potential resource leaks or unexpected behavior from multiple indexer threads operating on the same chain concurrently. Great fix!
• [INFO][style] The change from assert { type: 'json' } to with { type: 'json' } is a necessary update for JSON imports in ES Modules. This aligns with modern JavaScript module syntax and should be applied consistently across the codebase. Please ensure this change doesn't introduce any unexpected behavior in specific Node.js runtime environments, though Node.js v22 should support it well.
• [INFO][style] The reformatting of the nested ternary operator improves readability significantly. This is a positive style change.
• [INFO][other] Adding tearDownEnvironment in the after hook is good for test isolation and ensuring a clean state after test execution. This prevents side effects between test runs.
• [INFO][other] Adding setupEnvironment and tearDownEnvironment in configDatabase.test.ts significantly improves the reliability and isolation of these tests. This is a great practice for integration tests that modify shared resources.
• [INFO][style] Updating the types for config1 and config2 from any to OceanNodeConfig in oceanP2P.test.ts improves type safety and code clarity. Good change.
Fixes #1133
Changes proposed in this PR:
Node version from 20 to 22
Fix json imports
Fix race condition in indexer worker threads causing duplicate indexing in tests