chore: switch to pnpm and upgrade non-breaking packages #2464
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughCI workflow updated to Node 22.18.0 and switched from Yarn to PNPM. Documentation replaced Yarn commands with PNPM equivalents. package.json migrated to PNPM (packageManager set), Node/Volta versions updated, scripts changed to PNPM, and multiple dependencies upgraded, including adding @google-cloud/firestore. A test script comment was corrected. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
AnujChhikara
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/test.yml (1)
16-32: Upgrade core Actions versions and harden PNPM install (actionlint failures, cache, frozen lockfile).
actionlint flags v3 actions as too old; also add caching and a frozen lockfile to avoid accidental lock updates.Apply this diff:
strategy: matrix: node-version: [22.18.0] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} + cache: 'pnpm' - - name: Setup pnpm - uses: pnpm/action-setup@v2 + - name: Setup pnpm + uses: pnpm/action-setup@v4 with: version: 10.14.0 - - run: pnpm install + - run: pnpm install --frozen-lockfile - run: pnpm testCONTRIBUTING.md (1)
128-136: Update Node.js prerequisite in CONTRIBUTING.md
The CONTRIBUTING.md file still specifies “Node.js version 8.0 or higher,” but our package.json (engines.nodeat line 85) and CI workflows (22.18.0 in .github/workflows/test.yml) require Node.js 22.18.0. Please update both instances under “Pre-requisites” to match:• CONTRIBUTING.md line 128
• CONTRIBUTING.md line 135Suggested diff:
--- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -128,7 +128,7 @@ Pre-requisites: - - Node.js version 8.0 or higher. + - Node.js version 22.18.0 or higher. - Java version 1.8 or higher. @@ -135,7 +135,7 @@ Pre-requisites: - - Node.js version 8.0 or higher. + - Node.js version 22.18.0 or higher. - Java version 1.8 or higher.package.json (1)
1-94: Critical Security Vulnerabilities Detected — Urgent Dependency Updates RequiredAfter running
pnpm audit --prod && pnpm audit --dev, multiple high-severity issues were uncovered in both production and development dependencies. These must be addressed before merging:• jsonwebtoken (prod)
– Current:^8.5.1(vulnerable to legacy key usage, signature bypass, forgeable tokens)
– Upgrade to^9.0.0(patched in GHSA-8cf7-32gw-wr33, GHSA-hjrf-2m68-5959, GHSA-qwph-4952-7xr6)• express (prod)
– Current:~4.18.3(open-redirect, XSS viaredirect()and malformed URLs)
– Upgrade to^4.20.0or later (patched in GHSA-rv95-896h-c2vc, GHSA-qw6h-vgh9-j6wx)• body-parser (transitive)
– Vulnerable in versions<1.20.3(Denial-of-Service via URL encoding)
– Ensure>=1.20.3by bumping Express or adding a direct override (GHSA-qwcr-r2fm-qrc7)• path-to-regexp (transitive)
– Vulnerable in versions<0.1.12(ReDoS, backtracking regex)
– Ensure>=0.1.12via Express bump or direct override (GHSA-9wv6-86v2-598j, GHSA-rhx6-c78j-4q9w)• express-boom → boom → hoek (transitive)
–hoek <=6.1.3is subject to prototype pollution
– Upgrade to a boom release that uses a safehoek, or replace with@hapi/boom(GHSA-c429-5p7v-vgjp)• cross-spawn (dev via pre-commit)
– Versions<6.0.6vulnerable to ReDoS
– Upgrade to>=6.0.6(GHSA-3xgq-45jj-v275)• Other dev-tool transitive issues
–firebase-tools(express/body-parser/path-to-regexp/send/serve-static/cookie)
– Ensuresend >=0.19.0,serve-static >=1.16.0,cookie >=0.7.0, and bumpfirebase-toolsto a release with patched depsPlease update your
package.jsonaccordingly and rerun:pnpm audit --prod && pnpm audit --devuntil no high- or moderate-severity vulnerabilities remain.
Diff example (package.json):
--- a/package.json +++ b/package.json @@ dependencies -"jsonwebtoken": "^8.5.1", +"jsonwebtoken": "^9.0.0", -"express": "~4.18.3", +"express": "^4.20.0", @@ devDependencies - // ensure pre-commit’s cross-spawn is ≥6.0.6, or update pre-commit/tooling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.github/workflows/test.yml(1 hunks)CONTRIBUTING.md(4 hunks)README.md(5 hunks)package.json(1 hunks)scripts/tests/tdd.sh(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/test.yml
21-21: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
23-23: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 LanguageTool
README.md
[grammar] ~31-~31: Use correct spacing
Context: ... Development Please install pnpm and volta [Why Volta?](https://docs.volta.sh/guide/...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~44-~44: Make sure you are using the right part of speech
Context: ...ure that the pnpm-lock.yaml file is not update, you will need to use the --frozen-lock...
(QB_NEW_EN_OTHER_ERROR_IDS_21)
[grammar] ~44-~44: Use correct spacing
Context: ... need to use the --frozen-lockfile flag. shell pnpm install --frozen-lockfile #### Confirm correct configuration setup Thi...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~71-~71: Use correct spacing
Context: ...h #### Running a server in Dev mode shell pnpm dev ``` ## What happens in production: - Install p...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~77-~77: Use correct spacing
Context: ... dev ## What happens in production: - Install packages pnpm install ``` ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~79-~79: There might be a mistake here.
Context: ...ppens in production: - Install packages pnpm install - Run tests pnpm test - Prune de...
(QB_NEW_EN_OTHER)
[grammar] ~85-~85: There might be a mistake here.
Context: ...kages pnpm install - Run tests pnpm test - Prune dev dependencies ``` npm prune --...
(QB_NEW_EN_OTHER)
CONTRIBUTING.md
[grammar] ~4-~4: There might be a mistake here.
Context: ...tting-started) - pnpm Command Reference - Project Structure -...
(QB_NEW_EN)
[grammar] ~15-~15: Use correct spacing
Context: ...](README.md). ## pnpm Command Reference ##### pnpm install Installs all dependencies listed in th...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~19-~19: Use correct spacing
Context: ...cieslisted in the rootpackage.json. ##### pnpm test The script associated withpnpm test` w...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~23-~23: Make sure you are using the right part of speech
Context: ...ith pnpm test will run all tests that ensures that your commit does not break anythin...
(QB_NEW_EN_OTHER_ERROR_IDS_21)
[grammar] ~23-~23: There might be a mistake here.
Context: ...ur commit does not break anything in the repository. This will run the lint, inte...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...the repository. This will run the lint, integration and unit tests. ##### pnpm lint Run...
(QB_NEW_EN_OTHER)
[grammar] ~24-~24: Use correct spacing
Context: ...un the lint, integration and unit tests. ##### pnpm lint Runs the lint checks in the project. ##...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~28-~28: Use correct spacing
Context: ...t Runs the lint checks in the project. #####pnpm generate-api-schema Generates the API schema in the filepu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~32-~32: Use correct spacing
Context: ...ema in the file public/apiSchema.json. ##### pnpm validate-setup Runs the test for checking local develop...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~166-~166: There might be a mistake here.
Context: ...re done, the java process is not killed automatically and when our integration test run it gi...
(QB_NEW_EN_OTHER)
[grammar] ~166-~166: Make sure you are using the right part of speech
Context: ...matically and when our integration test run it gives error. - Error: connect ECONNR...
(QB_NEW_EN_OTHER_ERROR_IDS_21)
[grammar] ~166-~166: There might be a mistake here.
Context: ...ly and when our integration test run it gives error. - Error: connect ECONNREFUSED ::...
(QB_NEW_EN)
[grammar] ~166-~166: There might be a mistake here.
Context: ...our integration test run it gives error. - Error: connect ECONNREFUSED ::1:8081 ##...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
43-43: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
77-77: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
CONTRIBUTING.md
17-17: Heading levels should only increment by one level at a time
Expected: h3; Actual: h5
(MD001, heading-increment)
🔇 Additional comments (9)
scripts/tests/tdd.sh (1)
33-33: Doc tweak aligns with PNPM migration — looks good.
The note now correctly points to pnpm. No functional impact..github/workflows/test.yml (1)
18-18: Single-version matrix is fine; confirm alignment with engines/Volta.
Matrix pinned to 22.18.0 matches package.json engines and Volta. If you intend to keep multi-version CI in the future, consider adding LTS-1 as a second entry.CONTRIBUTING.md (1)
149-150: Windows commands updated to PNPM — looks good.
Matches package.json scripts and README guidance.package.json (6)
5-5: packageManager + Volta pin are consistent.
[email protected] matches Volta and CI config. Good.
9-9: Postinstall compiles TypeScript — confirm intended for production builds.
This will compile during install, before devDependencies are pruned. Fine for Heroku-like flows; ensure build environments with NODE_ENV=production still allow tsc to run.
16-16: test script chains lint/unit/integration — confirm emulator lifecycle.
Since integration follows unit, ensure emulator lifecycle (start/stop) is handled to avoid port conflicts (also referenced in CONTRIBUTING).
85-86: Engines match CI matrix.
Node 22.18.0 is aligned with GitHub Actions workflow and Volta pin.
90-93: Volta pin includes pnpm — nice.
This ensures contributors get the correct toolchain automatically.
20-48: Action Required: Confirm Behavior for Major Dependency Bumps (Helmet 8, Joi 18, Multer 2, Firebase-Admin 13)Please manually verify that none of these major upgrades introduced unintended behavioral changes in our codebase:
• Helmet (upgraded to v8) is configured in
middlewares/index.jsviaapp.use(helmet({ contentSecurityPolicy: false, dnsPrefetchControl: false, /* …other options… */ }));– Ensure the options you’ve disabled still exist and that no new default headers (or removed ones) affect API surface.
– Confirm that Helmet’s CommonJS import (require('helmet')) remains valid in v8.• Joi (upgraded to v18) is used extensively in
/middlewares/validators/**with patterns like:const schema = joi.object({ foo: joi.string().required().messages({ /* … */ }), bar: joi.boolean().optional(), /* … */ });– Verify that default validation behavior (e.g.
abortEarly, casting, date parsing) remains the same or is adjusted as expected.
– Spot-check a representative validator (e.g.middlewares/validators/events.js) to confirm custom messages still fire correctly.• Multer (upgraded to v2) is instantiated in
utils/multer.jsand applied in routes (routes/users.js,routes/badges.js):const multer = require('multer'); const upload = multer({ storage: multer.memoryStorage(), limits: { fileSize: profileFileSize } });– Confirm
memoryStorage()andlimits.fileSizeoptions are untouched in v2.
– Ensure yourinstanceof multer.MulterErrorchecks and manually thrownnew multer.MulterError('TYPE_UNSUPPORTED_FILE')still work as intended.• Firebase-Admin (upgraded to v13) is initialized in
utils/firestore.jsand imported elsewhere:const admin = require('firebase-admin'); const { Timestamp } = require('firebase-admin/firestore'); /* … */– Verify that the Firestore initialization (credentials,
admin.initializeApp()) and use ofTimestampremain compatible.
– Spot-check a service (e.g.services/impersonationRequests.ts) that usesTimestampto ensure no namespace or constructor changes.If you haven’t already, add or update unit/integration tests covering:
– API response headers generated by Helmet
– Joi validation failures (with custom messages)
– File‐upload errors (Multer limits and unsupported file types)
– Firestore reads/writes involvingTimestamp
|
We need to install @google-cloud/firestore because the latest version of Firestore Admin no longer includes it automatically, and pnpm requires all used packages to be listed directly in your project |
| pnpm test | ||
| ``` | ||
|
|
||
| - Prune dev dependencies |
There was a problem hiding this comment.
we do not need to prune dev dependencies we can directly install packages using pnpm install --prod in prod environment
railway-app
bot
temporarily deployed
to
website-backend (marvelous-patience / production)
railway-app
bot
temporarily deployed
to
triumphant-love (amiable-stillness / production)
Date: 20 Aug 2025
Developer Name: @AnujChhikara
Issue Ticket Number
Description
This PR migrates the project from Yarn to PNPM as the package manager and upgrades all packages that do not introduce breaking changes.
Successfully deployed to railway https://triumphant-love-production.up.railway.app/
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
screen-recording-2025-08-20-at-64322-pm_nfGFdzNM.mp4
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Switch from using Yarn to pnpm as the package manager, and upgrade several non-breaking package dependencies in the project.
Why are these changes being made?
Switching to pnpm aims to optimize package management with improved performance and disk space efficiency. Package upgrades ensure compatibility, security compliance, and access to new features, all without introducing breaking changes.