Repository navigation
feat(devvit): unify Vite build and Node oRPC server - #62
Conversation
|
Warning Review limit reached
Next review available in: 24 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds an opt-in official Devvit Vite build strategy, introduces a direct Node HTTP oRPC bridge adapter, migrates Devvit target servers from Express to native Node HTTP, and updates generated templates, validation, documentation, and Devvit package versions. ChangesDevvit build and bridge integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BuildTool
participant ViteConfig
participant DevvitPlugin
participant TargetServer
participant Bridge
BuildTool->>ViteConfig: Select devvit-vite strategy
ViteConfig->>DevvitPlugin: Build client and server bundles
DevvitPlugin->>TargetServer: Emit unified Devvit artifacts
TargetServer->>Bridge: Handle /api/mpgd/rpc
Bridge-->>TargetServer: Return oRPC response
Possibly related PRs
Suggested reviewers: Poem
🚥 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/validate-starter-workflow.ts (1)
862-978: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidation function is comprehensive and well-structured.
The new
validatePhaserTemplateDevvitVitePlugin()correctly checks:
- Required files across both wrapper roots
- Vite config markers (
@devvit/start/vite,createGameViteSharedConfig, conditional sourcemaps)- Server entry for direct oRPC Node adapter and legacy bridge compatibility
- Forbidden
express/helmetimports- Package version consistency with
expectedDevvitVersionmpgd.targets.jsonbuild strategy set todevvit-viteOne note: the forbidden-package list at line 915 includes
['express', 'helmet']but nothono. If Hono was also a removed dependency (as the READMEs suggest), consider adding it to the list. If the READMEs have a typo (should say "Helmet"), no change is needed here — this is tracked in the README review comments.🤖 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 `@tools/validate-starter-workflow.ts` around lines 862 - 978, Extend the forbiddenPackage list in validatePhaserTemplateDevvitVitePlugin to include hono if it was removed from the Phaser template dependencies. Preserve the existing import checks for express and helmet; make no change if Hono is intentionally retained and the README reference is incorrect.
🤖 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 `@apps/target-devvit/src/server/index.ts`:
- Around line 760-771: Update the oversized-body handling around
assertContentLengthWithinLimit so requests with a declared Content-Length above
maxBodySize are not left unread: after returning 413, either drain/discard the
IncomingMessage stream while marking the response Connection: close, or close
the socket after flushing the response. Preserve normal processing for requests
within the limit.
---
Nitpick comments:
In `@tools/validate-starter-workflow.ts`:
- Around line 862-978: Extend the forbiddenPackage list in
validatePhaserTemplateDevvitVitePlugin to include hono if it was removed from
the Phaser template dependencies. Preserve the existing import checks for
express and helmet; make no change if Hono is intentionally retained and the
README reference is incorrect.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39a28925-b9b8-4f85-90f1-a8d7e6d26207
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (43)
.sampo/changesets/devvit-vite-node-orpc.mdadapters/devvit/package.jsonapps/target-devvit/README.mdapps/target-devvit/devvit.jsonapps/target-devvit/package.jsonapps/target-devvit/src/client/game.htmlapps/target-devvit/src/client/game.tsapps/target-devvit/src/client/index.htmlapps/target-devvit/src/client/preview.tsapps/target-devvit/src/server/index.tsapps/target-devvit/tsconfig.jsonapps/target-devvit/vite.config.tsapps/target-devvit/vite.server.config.tsexamples/phaser-starter/mpgd.targets.jsonexamples/phaser-starter/vite.config.tsexamples/phaser-starter/vite.shared.tspackages/bridge/package.jsonpackages/bridge/src/cloudflare-pages.tspackages/bridge/src/orpc-node.test.tspackages/bridge/src/orpc-node.tspackages/bridge/src/orpc.tspackages/bridge/test/orpc-node-dist-import.mjspackages/bridge/tsconfig.jsonpackages/cli/templates/phaser-game/apps/target-devvit/README.mdpackages/cli/templates/phaser-game/apps/target-devvit/devvit.jsonpackages/cli/templates/phaser-game/apps/target-devvit/package.jsonpackages/cli/templates/phaser-game/apps/target-devvit/src/client/game.htmlpackages/cli/templates/phaser-game/apps/target-devvit/src/client/game.tspackages/cli/templates/phaser-game/apps/target-devvit/src/client/index.htmlpackages/cli/templates/phaser-game/apps/target-devvit/src/client/preview.tspackages/cli/templates/phaser-game/apps/target-devvit/src/server/index.tspackages/cli/templates/phaser-game/apps/target-devvit/tsconfig.jsonpackages/cli/templates/phaser-game/apps/target-devvit/vite.config.tspackages/cli/templates/phaser-game/apps/target-devvit/vite.server.config.tspackages/cli/templates/phaser-game/mpgd.targets.jsonpackages/cli/templates/phaser-game/vite.config.tspackages/cli/templates/phaser-game/vite.shared.tstools/target/build-target.tstools/target/platform-targets.tstools/target/schemas.tstools/target/validate-target-config.tstools/validate-starter-workflow.tstsconfig.base.json
💤 Files with no reviewable changes (2)
- packages/cli/templates/phaser-game/apps/target-devvit/vite.server.config.ts
- apps/target-devvit/vite.server.config.ts
|
@coderabbitai review Please re-review the latest commit |
|
✅ Action performedReview finished.
|
|
@coderabbitai The only actionable thread is now resolved and the suggested |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check open review threads/comments on the PR to confirm resolution status
gh pr view 62 --repo imjlk/mpgd-kit --json reviews,comments --jq '.reviews[] | {author: .author.login, state: .state, body: .body}' | head -100
echo "---THREADS---"
gh api graphql -f query='
query {
repository(owner: "imjlk", name: "mpgd-kit") {
pullRequest(number: 62) {
reviewThreads(first: 50) {
nodes {
isResolved
isOutdated
comments(first: 5) {
nodes {
author { login }
body
path
line
}
}
}
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | {isResolved: .isResolved, isOutdated: .isOutdated, path: .comments.nodes[0].path, body: .comments.nodes[0].body[0:200]}'Length of output: 8436 🏁 Script executed: #!/bin/bash
rg -n "hono" tools/validate-starter-workflow.ts -i
echo "---"
rg -n "forbiddenPackage" tools/validate-starter-workflow.ts -A5
echo "---content-length---"
rg -n "assertContentLengthWithinLimit|RequestBodyTooLargeError|Connection" apps/target-devvit/src/server/index.ts -B2 -A15Length of output: 4332 Confirmed. Both items are resolved:
I also re-checked all review threads via the GitHub API: the only actionable thread (oversized-body handling) is marked resolved, and no other open/unresolved actionable comments remain on the PR.
(=^･ω･^=) ﾉ ready to merge |
Summary
buildStrategy: "devvit-vite"target path that delegates client and server bundling to the official@devvit/start/viteplugin while preserving the legacy split-build path@mpgd/bridge/orpc/nodeadapter backed by@orpc/server/node, remove Express/Helmet and Fetch conversion from generated targets, and preserve the explicit legacy/api/mpgd/bridgerouteCompatibility
devvit-viteValidation
pnpm checkpnpm testpnpm build:packagespnpm pack:packages:preparedpnpm build:target reddit stagingpnpm build:target reddit productionpnpm smoke:target reddit*.mapartifactsKnown upstream warning
The official Devvit Vite plugin currently emits a non-fatal
sourcemapFileNamesoutput-option warning with Vite 8.1.3. Both staging and production builds complete successfully; production sourcemaps are explicitly disabled and verified absent.Summary by CodeRabbit
New Features
Dependency Updates
Changes