Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 203 files, which is 53 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (203)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
@m41denx check why vercel fails |
There was a problem hiding this comment.
Pull request overview
This PR appears to migrate the codebase to Nitro v3 (and updated H3/Nitro APIs), including updating runtime plugins/middleware, route handlers, and the Vitest setup/config to match the new event/runtime patterns.
Changes:
- Migrates server/runtime code to Nitro v3-style imports and plugin APIs (
nitro,nitro/h3,definePlugin, updated runtime config/storage usage). - Refactors many routes to run GDPS middleware explicitly and updates connector/controller call signatures to pass
H3Event. - Updates tooling/config: ESM mode (
"type": "module"), tsconfig path aliases, Vitest auto-import + global setup changes, and dependency bumps.
Reviewed changes
Copilot reviewed 199 out of 204 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.setup.ts | Converts Vitest global setup to default export returning teardown; adjusts test container injection. |
| vitest.config.ts | Updates config for Nitro test utils + aliases and auto-import output path. |
| tsconfig.json | Adds path aliases and changes Nitro tsconfig extends path. |
| types.d.ts | Updates module augmentations for Nitro/H3 context types. |
| package.json | Switches to ESM and bumps dependencies for Nitro v3 migration. |
| nitro.config*.ts | Updates Nitro configuration structure (aliases/serverDir/typescript generation/presets). |
| server/gdps_middleware/** | Refactors GDPS middleware initialization and helpers to accept H3Event and new helpers. |
| server/middleware/01.real_ip.ts | Updates client IP extraction logic for Nitro/H3 changes. |
| server/utils/usePostObject.ts | Changes form parsing approach and preparsed body caching. |
| controller/** | Updates controller typings and (in some places) runtime/config access patterns for Nitro v3. |
| server/plugins/** | Migrates plugins to Nitro v3 definePlugin, adjusts router/hooks usage, and storage integration. |
| server/routes/** | Refactors many route handlers for new middleware/event/connector patterns; adds headers across endpoints. |
| README.md | Updates license section and adds new README content. |
| .gitignore | Adds ignores for generated/compiled artifacts and test auto-import d.ts outputs. |
Comments suppressed due to low confidence (3)
server/routes/index.get.ts:29
- The Tailwind CDN URL contains an extra "?" ("https://cdn?.tailwindcss.com"), which will fail to load Tailwind in the browser. Replace it with the correct Tailwind CDN URL.
server/plugins/plugin-gdpsswitcher.ts:52 - The fallback icon URL includes an extra "?" ("https://cdn?.rigby.host/..."), which will result in a broken icon URL. Replace it with the correct host.
nitro.config-vercel.ts:49 - The comment URL for the Unstorage Vercel Blob driver is malformed ("https://unstorage?.unjs.io/..."), so the reference link is broken. Update it to the correct documentation URL.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export default defineEventHandler((event: H3Event) => { | ||
| const h = (header: string) => event.req.headers.get(header); | ||
| event.context.clientAddress = h("cf-connecting-ip") | ||
| || h("x-forwarded-for") | ||
| || h("x-real-ip") | ||
| || event.node.req.socket.remoteAddress; | ||
| || (event.node?.req?.socket?.remoteAddress); |
There was a problem hiding this comment.
event.req is not a stable/standard H3/Nitro API surface across runtimes; using event.req.headers.get(...) may throw if req is undefined. Prefer the H3 helpers (e.g., getRequestHeader) or event.node.req.headers (Node) / event.request.headers (web) consistently.
| export const withPreparsedForm = async (event: H3Event): Promise<FormData> => { | ||
| if (!event.context?._preparsedBody) | ||
| event.context._preparsedBody = await event.req.formData() | ||
| return event.context._preparsedBody as FormData |
There was a problem hiding this comment.
event.req.formData() is likely not available on H3Event in all Nitro runtimes (Node/CF/etc). This can crash request handling. Prefer readFormData(event) (h3 helper) or event.request.formData() if you are intentionally using the web Request API, and keep the approach consistent with the rest of the codebase.
| eq(scoresTable?.levelId, scoresTable?.levelId), | ||
| eq(scoresTable?.uid, scoresTable?.uid) |
There was a problem hiding this comment.
updateScore()'s WHERE clause compares each column to itself (levelId = levelId and uid = uid), which will match all rows and can update the entire table. This should constrain by the identifying values from data (or separate params), e.g. where(levelId = data.levelId AND uid = data.uid).
| eq(scoresTable?.levelId, scoresTable?.levelId), | |
| eq(scoresTable?.uid, scoresTable?.uid) | |
| eq(scoresTable?.levelId, data.levelId), | |
| eq(scoresTable?.uid, data.uid) |
| if (platform === "vercel") { | ||
| const storage = useStorage("config") | ||
| if (storage && typeof storage.mount === "function") { | ||
| storage.mount("config", storageDriver({ url: process.env.EDGE_CONFIG_TOKEN || "" })) | ||
| } |
There was a problem hiding this comment.
This mounts the "config" driver onto a storage instance that is already scoped to the "config" namespace (useStorage("config")), and then mounts again at key "config". This likely results in an incorrect mount point (double-prefix) or no-op depending on the storage wrapper. Use the root storage (useStorage()) when calling .mount("config", ...), or mount the driver via Nitro config instead of runtime mounting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 199 out of 204 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (useRuntimeConfig().platform) { | ||
| if (!privatePool) { | ||
| if (process.env.DATABASE_URL) { | ||
| if (process.env?.DATABASE_URL) { | ||
| console.log("Detected possible Postgres Neon") | ||
| privatePool = drizzle(process.env.DATABASE_URL, {schema, logger: !!process.env.VERBOSE}) | ||
| privatePool = drizzle(process.env?.DATABASE_URL, {schema, logger: !!process.env?.VERBOSE}) | ||
| } |
There was a problem hiding this comment.
useRuntimeConfig().platform is configured as a non-empty string (e.g. "vercel"/"cloudflare"/"standalone"), so if (useRuntimeConfig().platform) will always be truthy and force the code path that uses privatePool and env URLs, skipping per-srvid pools. Compare against the specific platforms that should use a single DB URL (or use a dedicated boolean flag).
| config.devStorage!.config = { | ||
| ...inject("config"), | ||
| driver: "redis" | ||
| } |
There was a problem hiding this comment.
This setup assumes config.devStorage exists, but nitro.config.ts no longer defines devStorage. The non-null assertion will crash tests at startup. Either reintroduce devStorage in the config used by tests, or update the injector/mocks to mount/override storage via useStorage().mount(...) (or another supported Nitro/unstorage mechanism).
| storage: { | ||
| savedata: { | ||
| driver: "s3", | ||
| accessKeyId: process.env.S3_ACCESS_KEY, | ||
| secretAccessKey: process.env.S3_SECRET, | ||
| endpoint: process.env.S3_URL, | ||
| bucket: process.env.S3_BUCKET, | ||
| region: process.env.S3_REGION || "us-east-1", | ||
| driver: "vercel-blob", | ||
| access: "public", // DO NOT CHANGE, THIS IS MANDATORY AND IS NOT A BUG: https://unstorage.unjs.io/drivers/vercel#vercel-blob | ||
| // token: process.env.BLOB_READ_WRITE_TOKEN, // Optional | ||
| }, | ||
| config: { | ||
| driver: "redis", | ||
| url: process.env.REDIS_URL, | ||
| } | ||
| // This driver doesn't exist in upstream unstorage, so it is loaded dynamically as storage plugin asn always | ||
| // needs process.env.EDGE_CONFIG | ||
| // config: { | ||
| // driver: "storage-vercel-edgeconfig", | ||
| // // url: process.env.EDGE_CONFIG // Optional | ||
| // } | ||
| }, |
There was a problem hiding this comment.
devStorage was removed from this config, but the vitest injector/mocks still import ~~/nitro.config and mutate/read config.devStorage.*. This will break tests unless the tests are switched to a config that still includes devStorage (e.g. the base config) or the tests are updated to mount drivers without relying on devStorage.
| const role = await event.context.user!.fetchRole() | ||
| if (role && role.privileges.aReqMod) { | ||
| await event.context.connector.numberedSuccess(role.modLevel,"Yes, you are a mod") | ||
| if (role && role.privileges?.aReqMod) { | ||
| await event.context.connector.numberedSuccess(event, role?.modLevel,"Yes, you are a mod") | ||
| } else { | ||
| await event.context.connector.error(-1, "You do not have permission to perform this action") | ||
| await event.context.connector.error(event, -1, "You do not have permission to perform this action") |
There was a problem hiding this comment.
The handler awaits connector.numberedSuccess() / connector.error() but does not return the resulting body string. Since the connector methods return response payloads (and don't call send here), this endpoint will respond with an empty body. Return the connector call in both branches.
| const date = quest.timeAdded | ||
| date.setHours(0, 0, 0, 0) | ||
| date.setDate(date.getDate() + (variant === "daily" ? 1 : 7)) | ||
|
|
||
| const left = Math.floor(Math.max(0, quest.timeAdded.getTime() - Date.now()) / 1000) | ||
| const left = Math.floor(Math.max(0, quest?.timeAdded.getTime() - Date.now()) / 1000) | ||
|
|
There was a problem hiding this comment.
left is computed from quest.timeAdded, which (given the controller query constraints) is not in the future, so left will always be 0. You already compute the next reset time in date; use date.getTime() (after adding 1/7 days) for the countdown instead.
| await event.context.connector?.quests.getSpecialLevel( | ||
| quest?.id, | ||
| left | ||
| ) |
There was a problem hiding this comment.
The handler awaits connector.quests.getSpecialLevel(...) but does not return it, so the route will respond with an empty body. Return the connector output (and avoid optional chaining here if connector is guaranteed by middleware).
| if (platform === "vercel") { | ||
| const storage = useStorage("config") | ||
| if (storage && typeof storage.mount === "function") { | ||
| storage.mount("config", storageDriver({ url: process.env.EDGE_CONFIG_TOKEN || "" })) | ||
| } |
There was a problem hiding this comment.
This mounts the driver onto useStorage("config") and then mounts another "config" namespace under it, which is likely the wrong level (and may be a no-op if the namespaced storage doesn't support mount). Mount drivers on the root storage (useStorage()) and mount the config namespace once.
| } | ||
|
|
||
| await event.context.connector.numberedSuccess(data.levelID, "Level uploaded successfully") | ||
| await event.context.connector.numberedSuccess(event, data?.levelID, "Level uploaded successfully") | ||
|
|
||
| } |
There was a problem hiding this comment.
This route awaits connector.numberedSuccess(...) but does not return it, so the response body will be empty. Return the connector call (and consider returning early in both the update and create branches for clarity).
| if (uid > 0) { | ||
| await event.context.connector.account.login(uid) | ||
| await new ActionController(event.context.drizzle) | ||
| .registerAction("login_user", 0, uid, {uname: data.userName}) | ||
| await event.context.connector?.account.login(uid) | ||
| await new ActionController(event.context?.drizzle) | ||
| .registerAction(event, "login_user", 0, uid, {uname: data?.userName}) | ||
|
|
There was a problem hiding this comment.
On successful login, the handler calls connector.account.login(uid) but does not return its result, so the response body will be empty. Return the connector output (and consider returning after registering the action to avoid falling through).
Nitro internal imports cannot resolve |
No description provided.