chore: update dependencies, add typescript - #3
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR migrates Valhalla API routes and client access flows to TypeScript, adds shared member authentication, updates Chakra UI integration, introduces UI utilities, and refreshes application, tooling, and repository workflow configuration. ChangesValhalla application migration
Repository workflow documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Wallet
participant Home
participant FilesRoute
participant MemberAuth
participant TheGraph
participant S3
Wallet->>Home: Connect and sign member message
Home->>FilesRoute: POST signature
FilesRoute->>MemberAuth: Fetch member addresses
MemberAuth->>TheGraph: Query DAO members
TheGraph-->>MemberAuth: Return member addresses
FilesRoute->>S3: List authorized files
S3-->>FilesRoute: Return object metadata
FilesRoute-->>Home: Return file list
Home->>FilesRoute: POST signature and file key
FilesRoute->>S3: Generate signed object URL
S3-->>FilesRoute: Return signed URL
FilesRoute-->>Home: Return channel URL
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
app/shared/Header.jsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
Stepsimport. All of these files import theStepscomponent from@chakra-ui/reactbut never use it.
app/shared/Header.jsx#L3-3: RemoveStepsfrom the destructuring import.components/ui/provider.tsx#L3-3: RemoveStepsfrom the destructuring import.components/ui/toaster.tsx#L3-11: RemoveStepsfrom the destructuring import.app/shared/Footer.jsx#L3-3: RemoveStepsfrom the destructuring import.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/shared/Header.jsx` at line 1, Remove the unused Steps symbol from the Chakra UI destructuring imports in Header.jsx, provider.tsx, toaster.tsx, and Footer.jsx, leaving all other imported symbols unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/channel/route.ts`:
- Around line 31-33: Move the user-signature verifyMessage call into the
existing try/catch in app/api/channel/route.ts lines 31-33 and
app/api/files/route.ts lines 32-35, ensuring malformed signatures are handled by
the routes’ existing graceful error response instead of escaping as unhandled
exceptions.
In `@app/api/files/route.ts`:
- Around line 39-40: Update the S3 listing flow around ListObjectsCommand so it
detects truncated responses and repeatedly requests subsequent pages using the
returned NextMarker or ContinuationToken until all objects are collected. Return
the complete aggregated contents in NextResponse.json while preserving the
existing response shape.
In `@app/api/shared/memberAuth.ts`:
- Around line 47-70: Update fetchMemberAddresses to paginate the GraphQL members
query rather than relying on the first: 400 limit. Repeatedly request subsequent
batches using the query’s skip/first parameters, accumulate all memberAddress
values, and stop when a batch is smaller than the page size; preserve lowercase
normalization in the returned array.
In `@app/config.ts`:
- Around line 7-14: Update the S3 configuration in s3Client to use undefined
when S3_ENDPOINT, S3_REGION, S3_KEY, or S3_SECRET is absent instead of
empty-string fallbacks, allowing the AWS SDK to resolve defaults and credentials
normally. Preserve the existing environment-variable mappings.
In `@app/page.tsx`:
- Around line 142-144: Update the files rendering map in the page component to
iterate over all files instead of files.slice(1), and exclude directory markers
by filtering keys that end with “/”. Preserve the existing key validation and
file rendering behavior for valid object keys.
In `@components/ui/color-mode.tsx`:
- Around line 81-105: Remove the duplicate colorPalette props from the Span
elements in LightMode and DarkMode, retaining only colorPalette="gray" and
leaving their existing classes and other props unchanged.
In `@components/ui/provider.tsx`:
- Around line 9-15: Integrate the exported Provider component into the
application root by updating app/layout.tsx to use it instead of the existing
Providers import, or move its ColorModeProvider wrapping logic into
app/providers.jsx. Ensure the active root provider tree includes
ColorModeProvider while preserving the existing ChakraProvider configuration and
application children.
In `@components/ui/tooltip.tsx`:
- Around line 1-2: Update the imports in the tooltip component to source Tooltip
directly from `@chakra-ui/react` rather than the local `@/components/ui/tooltip`
alias, preventing self-import recursion; also remove the unused Steps import.
In `@package.json`:
- Around line 6-10: Update the package scripts to replace the lint command in
the scripts block with the ESLint CLI invocation targeting the project root. Add
eslint and eslint-config-next to devDependencies, preserving the existing
Next.js lint configuration and other scripts.
In `@tsconfig.json`:
- Line 7: Update the TypeScript compiler option strict in tsconfig.json from
false to true so strict type checking, including strict null checks and
no-implicit-any validation, is enabled.
---
Nitpick comments:
In `@app/shared/Header.jsx`:
- Line 1: Remove the unused Steps symbol from the Chakra UI destructuring
imports in Header.jsx, provider.tsx, toaster.tsx, and Footer.jsx, leaving all
other imported symbols unchanged.
🪄 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: 2604234b-6569-46b5-80ab-fe0dc2b23797
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
.prettierignore.prettierrcapp/api/channel/route.jsapp/api/channel/route.tsapp/api/files/route.jsapp/api/files/route.tsapp/api/shared/memberAuth.tsapp/config.tsapp/layout.tsxapp/page.jsapp/page.tsxapp/providers.jsxapp/shared/Footer.jsxapp/shared/Header.jsxapp/utils/config.tsapp/utils/requests.jsapp/utils/requests.tscomponents/ui/color-mode.tsxcomponents/ui/provider.tsxcomponents/ui/toaster.tsxcomponents/ui/tooltip.tsxjsconfig.jsonnext.config.jspackage.jsonpnpm-workspace.yamltsconfig.json
💤 Files with no reviewable changes (5)
- app/api/files/route.js
- jsconfig.json
- app/utils/requests.js
- app/api/channel/route.js
- app/page.js
|
@ECWireless addressed all feedback, and seems to build on Vercel when I check. Somehow the build status here on the PR doesn't seem to be updated however. See that it's building correctly here Should be ready for review 🚀 |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Valhalla Next.js app by migrating core codepaths to TypeScript, upgrading dependencies/tooling (including a move to pnpm and a new ESLint flat config), and centralizing membership-gated access logic for the file/channel API routes. It also updates UI/provider utilities and improves per-channel loading UX in the main page.
Changes:
- Migrate key app code from JS to TS (pages, API routes, shared utilities) and add TypeScript tooling (
tsconfig.json). - Switch dependency/tooling setup (pnpm workspace, ESLint flat config, Prettier config, dependency upgrades).
- Introduce shared membership auth helper used by API routes and update UI/providers/components accordingly.
Reviewed changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds TypeScript compiler configuration for the app. |
| pnpm-workspace.yaml | Introduces pnpm workspace settings and build allowances. |
| package.json | Upgrades runtime deps and adds TS/ESLint/Prettier dev tooling; adjusts scripts. |
| next.config.js | Formatting/semicolons cleanup for Next config export. |
| jsconfig.json | Removes JS path config in favor of TypeScript setup. |
| eslint.config.mjs | Adds ESLint flat config using Next core-web-vitals + TS presets. |
| components/ui/tooltip.tsx | Adds Chakra-based Tooltip wrapper component. |
| components/ui/toaster.tsx | Adds Chakra toaster setup/component for app notifications. |
| components/ui/provider.tsx | Adds Chakra provider + color mode provider wiring. |
| components/ui/color-mode.tsx | Adds next-themes-based light/dark mode utilities and toggle button. |
| app/utils/requests.ts | Adds typed API request helpers with improved error propagation. |
| app/utils/requests.js | Removes JS version of API request helpers. |
| app/utils/config.ts | Updates wagmi config file formatting/TS migration. |
| app/shared/Footer.jsx | Formatting updates for Chakra footer component. |
| app/providers.jsx | Updates Chakra provider setup (new system-based config). |
| app/page.tsx | Rewrites home page in TS, adds react-query usage and per-channel loading state. |
| app/page.js | Removes JS home page implementation. |
| app/layout.tsx | Types RootLayout props and updates RainbowKit config handling. |
| app/config.ts | Adjusts S3 client env handling for TS migration. |
| app/api/shared/memberAuth.ts | Adds shared member auth helpers (request guards + member list fetch). |
| app/api/files/route.ts | Adds TS API route for listing files with membership gating. |
| app/api/files/route.js | Removes JS API route for listing files. |
| app/api/channel/route.ts | Adds TS API route for signed URL retrieval with membership gating. |
| app/api/channel/route.js | Removes JS API route for channel access. |
| .prettierrc | Adds (empty) Prettier configuration file. |
| .prettierignore | Adds Prettier ignore rules for artifacts (and components/ui). |
Comments suppressed due to low confidence (2)
app/layout.tsx:18
- Passing an empty string for projectId can lead to confusing runtime failures in RainbowKit/WalletConnect initialization. If NEXT_PUBLIC_PROJECT_ID is required, fail fast with an explicit error instead of silently substituting "".
app/config.ts:13 - Defaulting S3 endpoint/region/credentials to empty strings can mask missing configuration and produce hard-to-diagnose AWS errors. Prefer validating required env vars and surfacing a clear failure when they are unset.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pnpm-workspace.yaml`:
- Around line 11-16: Update the minimumReleaseAgeExclude configuration in
pnpm-workspace.yaml by either defining the intended minimumReleaseAge threshold
so these package exemptions are effective, or removing the unused exclusion list
if no release-age policy is required.
🪄 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 Plus
Run ID: 0aed78ea-e329-40b1-aa80-833ac7e79b9a
📒 Files selected for processing (11)
app/api/channel/route.tsapp/api/files/route.tsapp/page.tsxapp/shared/Footer.jsxcomponents/ui/color-mode.tsxcomponents/ui/provider.tsxcomponents/ui/toaster.tsxcomponents/ui/tooltip.tsxeslint.config.mjspackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- components/ui/toaster.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- components/ui/tooltip.tsx
- components/ui/provider.tsx
- app/api/channel/route.ts
- app/api/files/route.ts
- components/ui/color-mode.tsx
- app/page.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/api/files/route.ts (1)
22-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ValhallaFiletype duplicated across files.The same
ValhallaFileshape is independently declared here, inapp/page.tsx, and inapp/utils/requests.ts. Since this is the response contract for the S3 listing endpoint, consider exporting it once (e.g. from this route or a shared types module) and importing it in the client files to prevent the shapes drifting out of sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/files/route.ts` around lines 22 - 24, Centralize the ValhallaFile response type by exporting the existing declaration near the route handler, or from a shared types module, and remove the duplicate declarations from app/page.tsx and app/utils/requests.ts. Import and reuse that shared ValhallaFile type in both client files so the S3 listing response contract remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/shared/memberAuth.ts`:
- Around line 117-119: Remove the embedded Graph API key from the axios endpoint
in the member authentication request. Load the key from an environment variable
using the existing configuration pattern in app/config.ts, then construct the
The Graph URL with that value while preserving the endpoint and subgraph ID;
ensure the request handles the missing configuration consistently with other
required credentials.
---
Nitpick comments:
In `@app/api/files/route.ts`:
- Around line 22-24: Centralize the ValhallaFile response type by exporting the
existing declaration near the route handler, or from a shared types module, and
remove the duplicate declarations from app/page.tsx and app/utils/requests.ts.
Import and reuse that shared ValhallaFile type in both client files so the S3
listing response contract remains consistent.
🪄 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 Plus
Run ID: 1be3c92c-0aff-4825-ab6d-14111279393b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.gitignoreapp/api/channel/route.tsapp/api/files/route.tsapp/api/shared/memberAuth.tsapp/config.tsapp/page.tsxapp/utils/requests.tstsconfig.json
💤 Files with no reviewable changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
- tsconfig.json
- app/utils/requests.ts
- app/page.tsx
- app/api/channel/route.ts
app/page.tsx,app/api/*,app/utils/*,tsconfig.json).package-lock.jsonwith pnpm setup (pnpm-workspace.yaml) and updates project/tooling config (package.json,next.config.js, Prettier config files).app/api/shared/memberAuth.ts) to centralize access checks.components/ui/*,app/providers.jsx, layout/shared components).Why
Test Plan
pnpm install && pnpm devpnpm lintandpnpm tsc --noEmitSummary by CodeRabbit