Feature/f 013 single lockfile#281
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
@chiemezie1 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.gitignore (1)
27-29: LGTM — aligns with single-lockfile policy.Ignoring
package-lock.jsoncorrectly enforces pnpm as the sole lockfile at the repo level. As an optional hardening step, you might also consider ignoringyarn.lockto cover contributors who accidentally run yarn, and pairing this with a CI check (e.g., fail the build ifpackage-lock.jsonoryarn.lockis present) to meet the acceptance criterion "single lockfile policy enforced in CI" from issue#184.Optional tweak
next-env.d.ts package-lock.json +yarn.lock🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 27 - 29, Add an optional hardening change: update .gitignore to also ignore yarn.lock alongside package-lock.json and add a CI job (e.g., "single-lockfile" check) that fails the build if package-lock.json or yarn.lock are present in the repo; this ensures the repository enforces a single lockfile policy by both ignoring alternate lockfiles in .gitignore and explicitly rejecting them in CI.lib/api/client.ts (1)
21-55: Optional: tightenrequest()robustness while you're here.Non-blocking observations on the surrounding function (pre-existing, not introduced by this PR — address only if convenient):
await res.json()on line 44 can throw on malformed JSON bodies, bypassing the structuredApiErrorpath and surfacing aSyntaxErrorto callers. Wrapping it in try/catch and falling back to{ error: res.statusText }would make error handling uniform.DELETErequests can legitimately have bodies in some APIs;del()currently hard-codesundefined. Fine if the backend never needs it, but worth noting.- Given relevant snippets show
lib/api/user.ts::getReceiveQrcode()andapp/(app)/me/kyc/upload/page.tsxbypassrequest()with their ownfetch()calls, consider a follow-up to consolidate on a single client so future cross-cutting concerns (auth, tracing, retries) apply uniformly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/client.ts` around lines 21 - 55, The request() function can throw a raw SyntaxError if await res.json() fails; update request (function name: request) to wrap the JSON parse in try/catch: attempt res.json() when content-type includes application/json, but if parsing throws, set data = { error: res.statusText || 'Invalid JSON response', details: parseError } so the subsequent ApiError construction (type ApiError) uses that structured info; ensure non-OK responses still produce an ApiError with err.status = res.status and err.details populated from data (or the parse error) before throwing so callers always receive a consistent ApiError object even on malformed JSON.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/api/client.ts`:
- Around line 31-34: This hunk mixes an auth/security change into the lockfile
PR—revert the change in lib/api/client.ts so the CSRF cookie/XSRF-TOKEN logic is
restored and the new Authorization header behavior (the opts.token ->
headers['Authorization'] assignment) is removed from this commit; locate the
conditional around opts.token and the comment "CSRF cookie logic removed" and
restore the previous CSRF/XSRF handling code, moving any token/auth changes into
a separate focused PR with tests and backend coordination.
- Line 31: Confirm with the backend team whether any endpoints validate the
X-XSRF-TOKEN header and, if not, add a short top-of-file comment in
lib/api/client.ts (or an ADR) stating CSRF cookies/XSRF-TOKEN are intentionally
removed because the app uses Bearer Authorization headers; then refactor the
outlier getReceiveQrcode() in lib/api/user.ts to call the centralized request()
helper instead of a direct fetch so it receives the same auth and security
headers/behavior, and ensure request() preserves all required headers and error
handling for 419/403 responses.
---
Nitpick comments:
In @.gitignore:
- Around line 27-29: Add an optional hardening change: update .gitignore to also
ignore yarn.lock alongside package-lock.json and add a CI job (e.g.,
"single-lockfile" check) that fails the build if package-lock.json or yarn.lock
are present in the repo; this ensures the repository enforces a single lockfile
policy by both ignoring alternate lockfiles in .gitignore and explicitly
rejecting them in CI.
In `@lib/api/client.ts`:
- Around line 21-55: The request() function can throw a raw SyntaxError if await
res.json() fails; update request (function name: request) to wrap the JSON parse
in try/catch: attempt res.json() when content-type includes application/json,
but if parsing throws, set data = { error: res.statusText || 'Invalid JSON
response', details: parseError } so the subsequent ApiError construction (type
ApiError) uses that structured info; ensure non-OK responses still produce an
ApiError with err.status = res.status and err.details populated from data (or
the parse error) before throwing so callers always receive a consistent ApiError
object even on malformed JSON.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 807b19bc-0ae8-4c60-a973-5476ba928c20
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.gitignorelib/api/client.ts
|
@chiemezie1 resolve conflicts |
|
@Junman140 all conflicts fixed. Resolved merge conflicts and aligned with single lockfile policy (pnpm). |
2 participants
PR: Enforce Single Lockfile Policy (pnpm) (#184)
Summary:
Removes
package-lock.jsonto enforce pnpm as the sole package manager lockfile. This prevents dependency drift and ensures all contributors use the same dependency versions.Context:
Details:
package-lock.jsonfrom the repository root.pnpm-lock.yamlremains for dependency management.closes: #184
Summary by CodeRabbit